Re: [PATCH] Fix up rotate expansion

[Jakub Jelinek <jakub at redhat dot com>]

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  <jakub@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