On 31/01/2017 22:15, Segher Boessenkool wrote: > 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. No, just some hard regs whose natural mode size is 2 and UNIT_PER_WORD is 4... > > 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.
>> 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. OK, but not sure if it needs to be patched any more. Aurélien
