On Mon, May 19, 2014 at 11:30 AM, Thomas Preud'homme <thomas.preudho...@arm.com> wrote: >> From: Richard Biener [mailto:richard.guent...@gmail.com] >> >> Oh, and what happens for >> >> unsigned foo (unsigned char *x) >> { >> return x[0] << 24 | x[2] << 8 | x[3]; >> } >> >> ? We could do an unsigned int load from x and zero byte 3 >> with an AND. Enhancement for a followup, similar to also >> considering vector types for the load (also I'm not sure >> that uint64_type_node always has non-BLKmode for all >> targets). > > I implemented this but it makes the statistics more difficult to read > and thus the test more difficult to write. Indeed, when doing a full > bswap many of the intermediate computations are partial bswap. > I can get on with it by using a different string to notify of partial > bswap than full bswap and test the partial bswap feature in a > separate file to not get noise from the full bswap. However I still > don't like the idea of notifying for partial bswap in statements that > will eventually be eliminated as dead statements. Besides, this > occur because some recursion is made for each statement even > if they were already examined which is not very nice in itself.
Ah, indeed ... > The solution I see is to keep a map of visited statements but I > don't know if that's fine in this case. After all, it's not strictly > necessary as the pass would work without this. It would just serve > to avoid redundant computation and confusion statistics. ... having bswap cleanup after itself (thus remove dead code itself would be nice). Let's just keep the above as a possibility for further enhancements and focus on getting the current patch correct and committed. Btw, another enhancement is memory target. Thus recognize void bswap_mem (char *m1, char *m2) { m2[0] = m1[3]; m2[1] = m1[2]; m2[2] = m1[1]; m2[3] = m1[0]; } with all the complication that arises due to aliasing issues. Thanks, Richard. > Best regards, > > Thomas > >