[patch 79279] combine/simplify_set issue

Segher Boessenkool segher@kernel.crashing.org
Wed Feb 1 19:12:00 GMT 2017


Hi Aurelien,

On Wed, Feb 01, 2017 at 10:02:56AM +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.
> No, just some hard regs whose natural mode size is 2 and UNIT_PER_WORD
> is 4...

You said it is a paradoxical subreg?  Or do you mean the result is
a paradoxical subreg?

> > 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.
> OK, looks like both m1->m2 & m2 -> m1 checks would be needed, but the m1
> -> m2 should be filtererd by valid predicates (general_operand).
> Sorry about that.

Hrm, maybe you can show the RTL before and after this transform?

> >> 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.
> OK, but not sure if it needs to be patched any more.

Let's work that out then.


Segher



More information about the Gcc-patches mailing list