Re: [patch 79279] combine/simplify_set issue

[Segher Boessenkool <segher at kernel dot crashing dot org>]

Hello,

On Mon, Jan 30, 2017 at 10:43:23AM +0100, Aurelien Buhrig wrote:
> This patch fixes a combiner bug in simplify_set which calls
> CANNOT_CHANGE_MODE_CLASS with wrong mode params.
> It occurs when trying to simplify paradoxical subregs of hw regs (whose
> natural mode is lower than a word).
> 
> In fact, changing from (set x:m1 (subreg:m1 (op:m2))) to (set (subreg:m2
> x)  op2) is valid if REG_CANNOT_CHANGE_MODE_P (x, m1, m2) is false
> and REG_CANNOT_CHANGE_MODE_P (x, GET_MODE (src), GET_MODE (SUBREG_REG
> (src))

r62212 (in 2003) changed it to what we have now, it used to be what you
want to change it back to.

You say m1 > m2, which means you have WORD_REGISTER_OPERATIONS defined.

Where does this transformation go wrong?  Why is the resulting insn
recognised at all?  For example, general_operand should refuse it.
Maybe you have custom *_operand that do not handle subreg correctly?

The existing code looks correct: what we want to know is if an m2
interpreted as an m1 yields the correct value.  We might *also* want
your condition, but I'm not sure about that.

>    See:
> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=79279
> 
> Validated on a private target,
> bootstraped on x86_64-pc-linux-gnu, but I'm not sure which target is the
> most relevant for this patch though ...
> 
> OK to commit?

Sorry, no.  We're currently in development stage 4, and this is not a
regression (see <https://gcc.gnu.org/develop.html#stage4>).  But we can
of course discuss this further, and you can repost the patch when stage 1
opens (a few months from now) if you still want it.


Segher