On Fri, May 10, 2013 at 05:18:34PM -0600, Jeff Law wrote: > On 05/10/2013 08:53 AM, Jakub Jelinek wrote: > >Hi! > > > >Our rotate expansion if rotate optab isn't present for the respective > >mode looks unsafe for rotations by variable count if that count could > >be 0, because then it invokes right or left shift by bitsize. > >While on most targets the hw will probably do the right thing > >(it is fine if x << 32 will either yield x or 0, in both cases > >that ored together with x >> 0 aka x will still yield x), perhaps gcc > >could try to optimize based on the fact that undefined behavior can't > >happen, so IMHO it is better to generate a safer sequence. > > > >Ok for trunk? > > > >2013-05-10 Jakub Jelinek <ja...@redhat.com> > > > > * expmed.c (expand_shift_1): For rotations by const0_rtx just > > return shifted. Use (-op1) & (prec - 1) as other_amount > > instead of prec - op1.
> Found by inspection? Yes. > Presumably the rotate was synthesized by GCC > from some other set of operations. To be optimizable, we'd have to > prove the original sequence triggered undefined behaviour. See http://gcc.gnu.org/PR57157 and http://gcc.gnu.org/ml/gcc-patches/2013-05/msg00453.html Until recently only the bit_rotate: code in fold-const.c was pattern matching {L,R}ROTATE_EXPR and handled only the (x << y) {+|^} (x >> (b - y)) patterns which is indeed undefined behavior for y == 0. But since the above change trunk also handles the (x << y) {+|^} (x >> ((-y) & (b - 1)) pattern which is valid even for y = 0, thus the above patch adjusts what we generate. The info whether rotation count 0 was valid or not is unfortunately lost, adding two new {L,R}ROTATE0_EXPR tree codes might be overkill for this. > Seems that we ought to have a testcase, even though it probably > means scanning the tree dumps to pick up the undefined behaviour. > Approved with a testcase. I have added lots of testcases recently, for rotation by zero perhaps something similar to rotate-1a.c from above can be added as rotate-2b.c and rotate-4b.c, and test zero rotation. Jakub
