Re: [RFA][PR rtl-optimization/70263] Fix creation of new REG_EQUIV notes

[Jeff Law <law at redhat dot com>]

On 03/17/2016 12:23 PM, Bernd Schmidt wrote:
> On 03/17/2016 06:37 PM, Jeff Law wrote:
> > +  bitmap seen_insns;
> >
> > +  seen_insns = BITMAP_ALLOC (NULL);
>
> You could save an allocation here by making this a bitmap_head and using
> bitmap_initialize.
Done.

> > +      bitmap_set_bit (seen_insns, INSN_UID (insn));
> > +
> >        if (! INSN_P (insn))
> >      continue;
> >
> > @@ -3646,7 +3656,8 @@ update_equiv_regs (void)
> >        && ! find_reg_note (XEXP (reg_equiv[regno].init_insns, 0),
> >                    REG_EQUIV, NULL_RTX)
> >        && ! contains_replace_regs (XEXP (dest, 0))
> > -      && ! pdx_subregs[regno])
> > +      && ! pdx_subregs[regno]
> > +      && ! bitmap_bit_p (seen_insns, INSN_UID (insn)))
>
> This looks odd to me. Isn't this condition always false? Did you want to
> test the init_insn?
Right, we need to test that we found INIT_INSN prior to handling INSN. 
The test needs to go into the next conditional, after we initialize 
INIT_INSN.

I double-checked, we'll only set REG_BASIC_BLOCK if the set and all 
users are in the same block.  That was my recollection, but after the 
major goof in V1 of the patch, I wanted to be sure.

I also added a blurb to the dump file when we create these equivalences 
and included a test to verify the code fires.  I verified it fired on 
x86 and x86-64.  It may or may not fire on other targets, so I left the 
test in the i386 specific subdirectory.


Bootstrapped and regression tested on x86_64-linux-gnu.  And unlike the 
last version, it doesn't totally disable the note creation (as verified 
by the new test ;-)

OK for the trunk?

Thanks,
Jeff

diff --git a/gcc/ChangeLog b/gcc/ChangeLog
index b7711b8..6b57495 100644
--- a/gcc/ChangeLog
+++ b/gcc/ChangeLog
@@ -1,3 +1,12 @@
+2016-03-18  Jeff Law  <law@redhat.com>
+
+	PR rtl-optimization/70263
+	* ira.c (memref_used_between_p): Assert we found END in the insn chain.
+	(update_equiv_regs): When trying to move a store to after the insn
+	that sets the source of the store, make sure the store occurs after
+	the insn that sets the source of the store.  When successful note
+	the REG_EQUIV note created in the dump file.
+
 2016-03-17  H.J. Lu  <hongjiu.lu@intel.com>
 
 	PR driver/70192
diff --git a/gcc/ira.c b/gcc/ira.c
index 062b8a4..c12318a 100644
--- a/gcc/ira.c
+++ b/gcc/ira.c
@@ -3225,13 +3225,18 @@ memref_referenced_p (rtx memref, rtx x)
 }
 
 /* TRUE if some insn in the range (START, END] references a memory location
-   that would be affected by a store to MEMREF.  */
+   that would be affected by a store to MEMREF.
+
+   Callers should not call this routine if START is after END in the
+   RTL chain.  */
+
 static int
 memref_used_between_p (rtx memref, rtx_insn *start, rtx_insn *end)
 {
   rtx_insn *insn;
 
-  for (insn = NEXT_INSN (start); insn != NEXT_INSN (end);
+  for (insn = NEXT_INSN (start);
+       insn && insn != NEXT_INSN (end);
        insn = NEXT_INSN (insn))
     {
       if (!NONDEBUG_INSN_P (insn))
@@ -3245,6 +3250,7 @@ memref_used_between_p (rtx memref, rtx_insn *start, rtx_insn *end)
 	return 1;
     }
 
+  gcc_assert (insn == NEXT_INSN (end));
   return 0;
 }
 
@@ -3337,6 +3343,7 @@ update_equiv_regs (void)
   int loop_depth;
   bitmap cleared_regs;
   bool *pdx_subregs;
+  bitmap_head seen_insns;
 
   /* Use pdx_subregs to show whether a reg is used in a paradoxical
      subreg.  */
@@ -3606,11 +3613,14 @@ update_equiv_regs (void)
   /* A second pass, to gather additional equivalences with memory.  This needs
      to be done after we know which registers we are going to replace.  */
 
+  bitmap_initialize (&seen_insns, NULL);
   for (insn = get_insns (); insn; insn = NEXT_INSN (insn))
     {
       rtx set, src, dest;
       unsigned regno;
 
+      bitmap_set_bit (&seen_insns, INSN_UID (insn));
+
       if (! INSN_P (insn))
 	continue;
 
@@ -3651,6 +3661,7 @@ update_equiv_regs (void)
 	  rtx_insn *init_insn =
 	    as_a <rtx_insn *> (XEXP (reg_equiv[regno].init_insns, 0));
 	  if (validate_equiv_mem (init_insn, src, dest)
+	      && bitmap_bit_p (&seen_insns, INSN_UID (init_insn))
 	      && ! memref_used_between_p (dest, init_insn, insn)
 	      /* Attaching a REG_EQUIV note will fail if INIT_INSN has
 		 multiple sets.  */
@@ -3661,9 +3672,15 @@ update_equiv_regs (void)
 	      ira_reg_equiv[regno].init_insns
 		= gen_rtx_INSN_LIST (VOIDmode, insn, NULL_RTX);
 	      df_notes_rescan (init_insn);
+	      if (dump_file)
+		fprintf (dump_file,
+			 "Adding REG_EQUIV to insn %d for source of insn %d\n",
+			 INSN_UID (init_insn),
+			 INSN_UID (insn));
 	    }
 	}
     }
+  bitmap_clear (&seen_insns);
 
   cleared_regs = BITMAP_ALLOC (NULL);
   /* Now scan all regs killed in an insn to see if any of them are
diff --git a/gcc/testsuite/ChangeLog b/gcc/testsuite/ChangeLog
index 7fe7295..336de6a 100644
--- a/gcc/testsuite/ChangeLog
+++ b/gcc/testsuite/ChangeLog
@@ -1,3 +1,9 @@
+2016-03-18  Jeff Law  <law@redhat.com>
+
+	PR rtl-optimization/70263
+	* gcc.c-torture/compile/pr70263-1.c: New test.
+	* gcc.target/i386/pr70263-2.c: New test.
+
 2016-03-17  Jakub Jelinek  <jakub@redhat.com>
 
 	PR c++/70144
diff --git a/gcc/testsuite/gcc.c-torture/compile/pr70263-1.c b/gcc/testsuite/gcc.c-torture/compile/pr70263-1.c
new file mode 100644
index 0000000..d4bf280
--- /dev/null
+++ b/gcc/testsuite/gcc.c-torture/compile/pr70263-1.c
@@ -0,0 +1,11 @@
+int a[91];
+int b, c;
+void fn1() {
+  int n, m;
+  do {
+    a[c--];
+    a[--c] = m;
+    a[--m] = b;
+  } while (n);
+}
+
diff --git a/gcc/testsuite/gcc.target/i386/pr70263-2.c b/gcc/testsuite/gcc.target/i386/pr70263-2.c
new file mode 100644
index 0000000..18ebbf0
--- /dev/null
+++ b/gcc/testsuite/gcc.target/i386/pr70263-2.c
@@ -0,0 +1,23 @@
+/* { dg-do compile } */
+/* { dg-options "-O2 -fdump-rtl-ira" } */
+
+/* { dg-final { scan-rtl-dump "Adding REG_EQUIV to insn \[0-9\]+ for source of insn \[0-9\]+" "ira" } } */
+
+typedef float XFtype __attribute__ ((mode (XF)));
+typedef _Complex float XCtype __attribute__ ((mode (XC)));
+XCtype
+__mulxc3 (XFtype a, XFtype b, XFtype c, XFtype d)
+{
+  XFtype ac, bd, ad, bc, x, y;
+  ac = a * c;
+__asm__ ("": "=m" (ac):"m" (ac));
+  if (x != x)
+    {
+      _Bool recalc = 0;
+      if (((!(!(((ac) - (ac)) != ((ac) - (ac)))))))
+	recalc = 1;
+      if (recalc)
+	x = __builtin_huge_vall () * (a * c - b * d);
+    }
+  return x;
+}