[PATCH] combine: Fix up simplify_shift_const_1 for nested ROTATEs [PR97386]

Segher Boessenkool segher@kernel.crashing.org
Tue Oct 13 16:47:10 GMT 2020


Hi!

On Tue, Oct 13, 2020 at 09:44:25AM +0200, Jakub Jelinek wrote:
> The following testcases are miscompiled (the first one since my improvements
> to rotate discovery on GIMPLE, the other one for many years) because
> combiner optimizes nested ROTATEs with narrowing SUBREG in between (i.e.
> the outer rotate is performed in shorter precision than the inner one) to
> just one ROTATE of the rotated constant.  While that (under certain
> conditions) can work for shifts, it can't work for rotates where we can only
> do that with rotates of the same precision.

> OT, on the other side I wonder why the code doesn't handle ROTATERT.  While

ROTATERT is a relatively recent invention, and isn't handled in many
places.  I still think we would be better off without it, fwiw (anything
that can be expressed in RTL using ROTATERT can be expressed just as
easily with just ROTATE; it would help if me made ROTATE defined for
*all* rhs values though).

> earlier the code canonicalizes ROTATERT to ROTATE with the adjusted
> (constant) count, I mean if the inner op is ROTATERT, why can't it
> decanonicalize the outer one back to ROTATERT and treat it like that?

Many targets only support the canonical form, so this will likely
regress generated code quality?

> 	PR rtl-optimization/97386
> 	* combine.c (simplify_shift_const_1): Don't optimize nested ROTATEs if
> 	they have different modes.
> 
> 	* gcc.c-torture/execute/pr97386-1.c: New test.
> 	* gcc.c-torture/execute/pr97386-2.c: New test.

The patch is fine, okay for trunk, thanks!


Segher


More information about the Gcc-patches mailing list