diff mbox

PR79286, ira combine_and_move_insns in loops

Message ID 20170201134830.GA3731@bubble.grove.modra.org
State New
Headers show

Commit Message

Alan Modra Feb. 1, 2017, 1:48 p.m. UTC 
This patch cures PR79286 by restoring the REG_DEAD note test used
prior to r235660, but modified to only exclude insns that may trap.
I'd like to allow combine/move without a REG_DEAD note in loops
because insns in loops often lack such notes, and I recall seeing
quite a few cases at the time I wrote r235660 where loops benefited
from allowing the combine/move to happen.

I've been battling hardware instability on my x86_64 box all day, so
hopefully this finally passes bootstrap and regression testing
overnight.  OK to apply assuming no regressions?

	PR rtl-optimization/79286
	* ira.c (combine_and_move_insns): Don't combine or move when
	use_insn does not have a REG_DEAD note and def_insn may trap.
testsuite/
	* gcc.c-torture/execute/pr79286.c: New.

Comments

Alan Modra Feb. 1, 2017, 9:37 p.m. UTC | #1 
On Thu, Feb 02, 2017 at 12:18:31AM +1030, Alan Modra wrote:
> This patch cures PR79286 by restoring the REG_DEAD note test used
> prior to r235660, but modified to only exclude insns that may trap.
> I'd like to allow combine/move without a REG_DEAD note in loops
> because insns in loops often lack such notes, and I recall seeing
> quite a few cases at the time I wrote r235660 where loops benefited
> from allowing the combine/move to happen.

Ugh, the new testcase fails for x86 -m32 -Os, but not due to ira this
time but rather reload.  I haven't looked into what is going wrong in
reload yet, but the net result is the same:  The faulting mem read is
moved before the printf call.

There were no other testsuite regressions, apart from the random set
of fails I have been getting for a long time on x86_64 for
c-c++-common/ubsan/float-cast-overflow-10.c,
c-c++-common/ubsan/float-cast-overflow-2.c,
c-c++-common/ubsan/float-cast-overflow-8.c, and
c-c++-common/ubsan/overflow-mul-4.c.

What is the correct thing to do for a new testcase that fails like
this?  Add a dg-fail-if?  Assuming I or someone else can't fix the
reload fail.

The new testcase -Os failure occurs on gcc-4.x, gcc-5 and gcc-6, but
gcc-3.4 passes.
diff mbox

Patch

diff --git a/gcc/ira.c b/gcc/ira.c
index 96b4b62..cdde775 100644
--- a/gcc/ira.c
+++ b/gcc/ira.c
@@ -3682,6 +3682,14 @@  combine_and_move_insns (void)
       gcc_assert (DF_REG_DEF_COUNT (regno) == 1 && DF_REF_INSN_INFO (def));
       rtx_insn *def_insn = DF_REF_INSN (def);
 
+      /* Even though we know this reg is set exactly once and used
+	 exactly once, check that the reg dies in the use insn or that
+	 the def insn can't trap.  This is to exclude degenerate cases
+	 in loops where the use occurs before the def.  See PR79286.  */
+      if (!find_reg_note (use_insn, REG_DEAD, regno_reg_rtx[regno])
+	  && may_trap_p (PATTERN (def_insn)))
+	continue;
+
       /* We may not move instructions that can throw, since that
 	 changes basic block boundaries and we are not prepared to
 	 adjust the CFG to match.  */
diff --git a/gcc/testsuite/gcc.c-torture/execute/pr79286.c b/gcc/testsuite/gcc.c-torture/execute/pr79286.c
new file mode 100644
index 0000000..e6d0e93
--- /dev/null
+++ b/gcc/testsuite/gcc.c-torture/execute/pr79286.c
@@ -0,0 +1,15 @@ 
+int a = 0, c = 0;
+static int d[][8] = {};
+
+int main ()
+{
+  int e;
+  for (int b = 0; b < 4; b++)
+    {
+      __builtin_printf ("%d\n", b, e);
+      while (a && c++)
+	e = d[300000000000000000][0];
+    }
+
+  return 0;
+}