| Message ID | 20170309211217.GL22703@tucnak |
|---|---|
| State | New |
| Headers | show |
Hi Jakub, On Thu, Mar 09, 2017 at 10:12:17PM +0100, Jakub Jelinek wrote: > The following testcase fails on ppc* due to invalid RTL sharing. > The problem is that replace_rtx never copy_rtx or copy_rtx_if_shared > anything, so if i{2,1,0}dest is present in CALL_INSN_FUNCTION_USAGE more > than once, and corresponding i{2,1,0}src is non-shareable RTL, then we ICE. > On the other side, copying whole CALL_INSN_FUNCTION_USAGE just in case, even > if we don't make any changes in there seems like a waste to me, furthermore, > I think the checking should make sure that CALL_INSN_FUNCTION_USAGE is never > shared between multiple instructions, so it shouldn't be a problem to modify > it in place. And lastly, if say i2dest appears in (plus i2dest const_int) > and i2src is (plus reg const_int), we don't simplify that into a single > plus. > > The following patch should fix that, by not copying the EXPR_LIST nodes at > all, and just using simplify_replace_rtx which will not modify if no change > is needed, and will copy_rtx the src whenever a replacement is made. > > Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk? Okay, thanks! Segher
--- gcc/combine.c.jj 2017-02-13 16:39:09.000000000 +0100 +++ gcc/combine.c 2017-03-09 11:17:11.733117350 +0100 @@ -4293,26 +4293,25 @@ try_combine (rtx_insn *i3, rtx_insn *i2, if (CALL_P (i3) && CALL_INSN_FUNCTION_USAGE (i3)) { - rtx call_usage = CALL_INSN_FUNCTION_USAGE (i3); - - reset_used_flags (call_usage); - call_usage = copy_rtx (call_usage); - - if (substed_i2) + for (rtx link = CALL_INSN_FUNCTION_USAGE (i3); link; + link = XEXP (link, 1)) { - /* I2SRC must still be meaningful at this point. Some splitting - operations can invalidate I2SRC, but those operations do not - apply to calls. */ - gcc_assert (i2src); - replace_rtx (call_usage, i2dest, i2src); + if (substed_i2) + { + /* I2SRC must still be meaningful at this point. Some + splitting operations can invalidate I2SRC, but those + operations do not apply to calls. */ + gcc_assert (i2src); + XEXP (link, 0) = simplify_replace_rtx (XEXP (link, 0), + i2dest, i2src); + } + if (substed_i1) + XEXP (link, 0) = simplify_replace_rtx (XEXP (link, 0), + i1dest, i1src); + if (substed_i0) + XEXP (link, 0) = simplify_replace_rtx (XEXP (link, 0), + i0dest, i0src); } - - if (substed_i1) - replace_rtx (call_usage, i1dest, i1src); - if (substed_i0) - replace_rtx (call_usage, i0dest, i0src); - - CALL_INSN_FUNCTION_USAGE (i3) = call_usage; } if (undobuf.other_insn) --- gcc/testsuite/gcc.target/powerpc/pr79909.c.jj 2017-03-09 11:46:06.088145481 +0100 +++ gcc/testsuite/gcc.target/powerpc/pr79909.c 2017-03-09 11:46:49.535573450 +0100 @@ -0,0 +1,13 @@ +/* PR rtl-optimization/79909 */ +/* { dg-do compile } */ +/* { dg-options "-O2 -mxl-compat" } */ + +typedef float T __attribute__ ((mode (TD))); +T b, c, d, e, f, g; +void bar (T, T, T, T, T, T); + +void +foo (void) +{ + bar (e, b, f, c, g, d); +}
Hi! The following testcase fails on ppc* due to invalid RTL sharing. The problem is that replace_rtx never copy_rtx or copy_rtx_if_shared anything, so if i{2,1,0}dest is present in CALL_INSN_FUNCTION_USAGE more than once, and corresponding i{2,1,0}src is non-shareable RTL, then we ICE. On the other side, copying whole CALL_INSN_FUNCTION_USAGE just in case, even if we don't make any changes in there seems like a waste to me, furthermore, I think the checking should make sure that CALL_INSN_FUNCTION_USAGE is never shared between multiple instructions, so it shouldn't be a problem to modify it in place. And lastly, if say i2dest appears in (plus i2dest const_int) and i2src is (plus reg const_int), we don't simplify that into a single plus. The following patch should fix that, by not copying the EXPR_LIST nodes at all, and just using simplify_replace_rtx which will not modify if no change is needed, and will copy_rtx the src whenever a replacement is made. Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk? 2017-03-09 Jakub Jelinek <jakub@redhat.com> PR rtl-optimization/79909 * combine.c (try_combine): Use simplify_replace_rtx on individual CALL_INSN_FUNCTION_USAGE elements instead of replace_rtx on copy_rtx of the whole CALL_INSN_FUNCTION_USAGE. * gcc.target/powerpc/pr79909.c: New test. Jakub