On 29/11/2016 11:16, Richard Biener wrote: >>> >> (bit_and >>> >> (if (shift > 0) >>> >> (lshift (convert @0) { build_int_cst (integer_type_node, shift); }) >>> >> (convert (rshift @0 { build_int_cst (integer_type_node, -shift); }))) >>> >> @3) >>> >> >>> >> What do you think? >> > >> > Shouldn't be necessary if we're working on unsigned types. May be >> > necessary >> > on signed types unless we're 100% sure sign bit tests will be >> > canonicalized. > Instead of the bit_and better put a if() condition around this.
Note that the bit_and is not duplicated. While v1 of the patch did and-then-shift (the source BIT_AND_EXPR was @2), here I'm doing shift-then-and so I would stop capturing the source BIT_AND_EXPR. It also matches what I'm doing for the A < 0 ? D : C pattern, so I think it's nicer this way. (BTW the above of course doesn't work because (if...) is only allowed at the top level, but I've bootstrapped and regtested already a version that works). Paolo