https://gcc.gnu.org/bugzilla/show_bug.cgi?id=70245

--- Comment #12 from Uroš Bizjak <ubizjak at gmail dot com> ---
(In reply to Jakub Jelinek from comment #10)
> (In reply to Uroš Bizjak from comment #9)
> > Please disable the pattern for gcc-6, and let's fix replace_rtx for gcc-7.
> 
> Disabling the peephole2 entirely would be too aggressive, if we want to
> avoid the problems with that, we could as well just guard it with
> && !reg_overlap_mentioned_p (operands[0], operands[2])
> or perhaps even allow simple cases like:
> && (!reg_overlap_mentioned_p (operands[0], operands[2])
>     || rtx_equal_p (operands[0], XEXP (operands[2], 0)))
> and in that case of simple REG just replace_equiv_address.
> But, the simplify_replace_rtx patch I've attached will do pretty much the
> same, it will try to do the peephole2, and punt if the result is not valid
> address.
> simplify_replace_rtx doesn't simplify anything if there is no change, so
> e.g. the
> !reg_overlap_mentioned_p (operands[0], operands[2]) case would always work
> as it used to.

I'm OK with the proposed patch as an interim solution, too - but please add a
comment referring to this PR why special handling is needed.

> Though, I'm worried about all the other replace_rtx uses in the backends:
> config/epiphany/epiphany.md:  replace_rtx (operands[2], operands[9],
> operands[3]);
> config/epiphany/epiphany.md:  replace_rtx (operands[2], operands[0],
> operands[10]);
> config/i386/i386.c:   replace_rtx (DF_REF_INSN (ref), reg, scopy);
> config/rl78/rl78.c:             OP (op) = replace_rtx (OP (op), gen_rtx_REG 
> (HImode,
> SP_REG), HL);
> config/rs6000/rs6000.c:    real = replace_rtx (real, reg2, rreg);
> config/rs6000/rs6000.c:    real = replace_rtx (real, reg,
> config/sh/sh.md:  replace_rtx (operands[4], operands[0], operands[1]);
> config/xtensa/xtensa.c:                 PATTERN (insn) = replace_rtx 
> (copy_rtx (PATTERN
> (insn)),
> 
> The i386.c one is fine, reg is a pseudo, and there should be pointer
> equivalency for pseudos I think.  What about all the others?

It looks to me that the intention of replace_rtx is to replace all occurences
of equal RTXes. There is no point of leaving some equals unchanged, as they
will probably be clobbered, as is the case with the attached test. But trying
to stay on the safe side, the infrastructure change is indeed too risky ATM.

Reply via email to