Hi!
On Wed, Mar 16, 2016 at 05:48:33PM -0500, Segher Boessenkool wrote:
> On Wed, Mar 16, 2016 at 01:59:29PM +0100, Bernd Schmidt wrote:
> > On 03/16/2016 01:22 PM, Jakub Jelinek wrote:
> > >So, this is what we've converged to on IRC and passed bootstrap/regtest
> > >on x86_64-linux and i686-linux. Is this ok for trunk?
> >
> > The explanation was a bit confusing at first, but I think this looks
> > reasonable. The assert worries me, but triggering it would indicate a
> > potential miscompilation, so I think it is preferrable to have it.
>
> This caused PR70261. Maybe it's just the assert.
Unfortunately it is not just the assert, and it is on various targets.
On powerpc64, one issue is during combiner, where we most likely don't
strictly care if we replace all, but can't replace one with bad mode and
can run into that case, and then during prologue threading, where
the rs6000 backend for whatever strange reason I haven't understood
really wants pointer equality instead of REGNO comparison (even when the
modes match), one (reg:DI 12) should be replaced, another (reg:DI 12)
should not.
Thus, I've reverted the patch (kept the testcase), and after some
discussions on IRC bootstrapped/regtested on x86_64-linux and i686-linux
following version, which right now should change behavior just for the i?86
case and nothing else, so shouldn't break other targets.
I believe at least the epiphany and sh peepholes that use replace_rtx will
want similar treatment, but will leave testing of that to their maintainers.
Ok for trunk?
2016-03-17 Jakub Jelinek <ja...@redhat.com>
PR target/70245
* rtl.h (replace_rtx): Add ALL_REGS argument.
* rtlanal.c (replace_rtx): Likewise. If true, use REGNO
equality and assert mode is the same, instead of just rtx pointer
equality.
* config/i386/i386.md (mov + arithmetics with load peephole): Pass
true as ALL_REGS argument to replace_rtx.
--- gcc/rtl.h.jj 2016-03-16 10:36:36.000000000 +0100
+++ gcc/rtl.h 2016-03-17 10:05:53.144242638 +0100
@@ -3011,7 +3011,7 @@ extern bool can_nonlocal_goto (const rtx
extern void copy_reg_eh_region_note_forward (rtx, rtx_insn *, rtx);
extern void copy_reg_eh_region_note_backward (rtx, rtx_insn *, rtx);
extern int inequality_comparisons_p (const_rtx);
-extern rtx replace_rtx (rtx, rtx, rtx);
+extern rtx replace_rtx (rtx, rtx, rtx, bool = false);
extern void replace_label (rtx *, rtx, rtx, bool);
extern void replace_label_in_insn (rtx_insn *, rtx, rtx, bool);
extern bool rtx_referenced_p (const_rtx, const_rtx);
--- gcc/rtlanal.c.jj 2016-03-17 08:59:37.000000000 +0100
+++ gcc/rtlanal.c 2016-03-17 10:16:42.870241836 +0100
@@ -2946,10 +2946,13 @@ inequality_comparisons_p (const_rtx x)
not enter into CONST_DOUBLE for the replace.
Note that copying is not done so X must not be shared unless all copies
- are to be modified. */
+ are to be modified.
+
+ ALL_REGS is true if we want to replace all REGs equal to FROM, not just
+ those pointer-equal ones. */
rtx
-replace_rtx (rtx x, rtx from, rtx to)
+replace_rtx (rtx x, rtx from, rtx to, bool all_regs)
{
int i, j;
const char *fmt;
@@ -2961,9 +2964,17 @@ replace_rtx (rtx x, rtx from, rtx to)
if (x == 0)
return 0;
- if (GET_CODE (x) == SUBREG)
+ if (all_regs
+ && REG_P (x)
+ && REG_P (from)
+ && REGNO (x) == REGNO (from))
+ {
+ gcc_assert (GET_MODE (x) == GET_MODE (from));
+ return to;
+ }
+ else if (GET_CODE (x) == SUBREG)
{
- rtx new_rtx = replace_rtx (SUBREG_REG (x), from, to);
+ rtx new_rtx = replace_rtx (SUBREG_REG (x), from, to, all_regs);
if (CONST_INT_P (new_rtx))
{
@@ -2979,7 +2990,7 @@ replace_rtx (rtx x, rtx from, rtx to)
}
else if (GET_CODE (x) == ZERO_EXTEND)
{
- rtx new_rtx = replace_rtx (XEXP (x, 0), from, to);
+ rtx new_rtx = replace_rtx (XEXP (x, 0), from, to, all_regs);
if (CONST_INT_P (new_rtx))
{
@@ -2997,10 +3008,11 @@ replace_rtx (rtx x, rtx from, rtx to)
for (i = GET_RTX_LENGTH (GET_CODE (x)) - 1; i >= 0; i--)
{
if (fmt[i] == 'e')
- XEXP (x, i) = replace_rtx (XEXP (x, i), from, to);
+ XEXP (x, i) = replace_rtx (XEXP (x, i), from, to, all_regs);
else if (fmt[i] == 'E')
for (j = XVECLEN (x, i) - 1; j >= 0; j--)
- XVECEXP (x, i, j) = replace_rtx (XVECEXP (x, i, j), from, to);
+ XVECEXP (x, i, j) = replace_rtx (XVECEXP (x, i, j),
+ from, to, all_regs);
}
return x;
--- gcc/config/i386/i386.md.jj 2016-03-16 10:36:36.000000000 +0100
+++ gcc/config/i386/i386.md 2016-03-17 10:09:01.281636259 +0100
@@ -17885,7 +17885,7 @@ (define_peephole2
(parallel [(set (match_dup 0)
(match_op_dup 3 [(match_dup 0) (match_dup 1)]))
(clobber (reg:CC FLAGS_REG))])]
- "operands[4] = replace_rtx (operands[2], operands[0], operands[1]);")
+ "operands[4] = replace_rtx (operands[2], operands[0], operands[1], true);")
(define_peephole2
[(set (match_operand 0 "register_operand")
Jakub
