Patchwork Fix PR 51505

login
register
mail settings
Submitter Andrey Belevantsev
Date Jan. 18, 2012, 4:41 p.m.
Message ID <4F16F628.8030002@ispras.ru>
Download mbox | patch
Permalink /patch/136659/
State New
Headers show

Comments

Andrey Belevantsev - Jan. 18, 2012, 4:41 p.m.
Hello,

As discussed in Bugzilla, this is the patch implementing Paolo's suggestion 
of killing REG_EQUAL/REG_EQUIV notes from df_kill_notes.  The code assumes 
there is at most one such note per insn.  Bootstrapped and tested on 
x86-64, ok for trunk?

Andrey

gcc:
2012-01-18  Andrey Belevantsev  <abel@ispras.ru>

	PR rtl-optimization/51505
	* df-problems.c (df_kill_notes): New parameter live.  Update comment.
	Remove REG_EQUAL/REG_EQUIV notes referring to dead registers.
	(df_note_bb_compute): Update the call to df_kill_notes.

testsuite:
2012-01-18  Andrey Belevantsev  <abel@ispras.ru>

	PR rtl-optimization/51505
	* gcc.dg/pr51505.c: New test.
Paolo Bonzini - Jan. 18, 2012, 5:28 p.m.
On 01/18/2012 05:41 PM, Andrey Belevantsev wrote:
> Hello,
>
> As discussed in Bugzilla, this is the patch implementing Paolo's
> suggestion of killing REG_EQUAL/REG_EQUIV notes from df_kill_notes. The
> code assumes there is at most one such note per insn. Bootstrapped and
> tested on x86-64, ok for trunk?
>
> Andrey
>
> gcc:
> 2012-01-18 Andrey Belevantsev <abel@ispras.ru>
>
> PR rtl-optimization/51505
> * df-problems.c (df_kill_notes): New parameter live. Update comment.
> Remove REG_EQUAL/REG_EQUIV notes referring to dead registers.
> (df_note_bb_compute): Update the call to df_kill_notes.
>
> testsuite:
> 2012-01-18 Andrey Belevantsev <abel@ispras.ru>
>
> PR rtl-optimization/51505
> * gcc.dg/pr51505.c: New test.

Ok, thanks for working on this.

Paolo
Andrey Belevantsev - Jan. 19, 2012, 7:32 a.m.
On 18.01.2012 21:28, Paolo Bonzini wrote:
> On 01/18/2012 05:41 PM, Andrey Belevantsev wrote:
> Ok, thanks for working on this.
Installed, do you want this for 4.6/4.5?

Andrey
Jakub Jelinek - Jan. 19, 2012, 7:34 a.m.
On Thu, Jan 19, 2012 at 11:32:41AM +0400, Andrey Belevantsev wrote:
> On 18.01.2012 21:28, Paolo Bonzini wrote:
> >On 01/18/2012 05:41 PM, Andrey Belevantsev wrote:
> >Ok, thanks for working on this.
> Installed, do you want this for 4.6/4.5?

If yes, please give it at least a couple of weeks on the trunk.

	Jakub
Paolo Bonzini - Jan. 19, 2012, 8:44 a.m.
On 01/19/2012 08:34 AM, Jakub Jelinek wrote:
>>> >  >Ok, thanks for working on this.
>> >  Installed, do you want this for 4.6/4.5?
> If yes, please give it at least a couple of weeks on the trunk.

It's fine by me but yes, let's give it time to bake.

Paolo
Eric Botcazou - Jan. 29, 2012, 1:02 p.m.
> As discussed in Bugzilla, this is the patch implementing Paolo's suggestion
> of killing REG_EQUAL/REG_EQUIV notes from df_kill_notes.  The code assumes
> there is at most one such note per insn.

That's wrong though and wreaks havoc during reload, e.g.:

(insn 169 60 62 4 (set (reg:TF 158)
        (mem/c:TF (plus:SI (reg/f:SI 101 %sfp)
                (const_int -16 [0xfffffffffffffff0])) [3 S16 A64])) 
960513-1.c:13 97 {*movtf_insn_sp32}
     (expr_list:REG_EQUIV (mem/c:TF (plus:SI (reg/f:SI 101 %sfp)
                (const_int -16 [0xfffffffffffffff0])) [3 S16 A64])
        (expr_list:REG_EQUAL (mult:TF (reg/v:TF 110 [ d ])
                (reg:TF 154))
            (nil))))

because the REG_EQUIV note disappears behind reload's back and it isn't 
prepared for that.  This is the cause of the following regression on SPARC:

FAIL: gcc.c-torture/execute/960513-1.c execution,  -Os
Eric Botcazou - Jan. 29, 2012, 3:09 p.m.
> > As discussed in Bugzilla, this is the patch implementing Paolo's
> > suggestion of killing REG_EQUAL/REG_EQUIV notes from df_kill_notes.  The
> > code assumes there is at most one such note per insn.
>
> That's wrong though and wreaks havoc during reload, e.g.:
>
> (insn 169 60 62 4 (set (reg:TF 158)
>         (mem/c:TF (plus:SI (reg/f:SI 101 %sfp)
>                 (const_int -16 [0xfffffffffffffff0])) [3 S16 A64]))
> 960513-1.c:13 97 {*movtf_insn_sp32}
>      (expr_list:REG_EQUIV (mem/c:TF (plus:SI (reg/f:SI 101 %sfp)
>                 (const_int -16 [0xfffffffffffffff0])) [3 S16 A64])
>         (expr_list:REG_EQUAL (mult:TF (reg/v:TF 110 [ d ])
>                 (reg:TF 154))
>             (nil))))
>
> because the REG_EQUIV note disappears behind reload's back and it isn't
> prepared for that.  This is the cause of the following regression on SPARC:
>
> FAIL: gcc.c-torture/execute/960513-1.c execution,  -Os

As well as:

FAIL: gcc.c-torture/execute/stdarg-2.c execution,  -O2 
FAIL: gcc.c-torture/execute/stdarg-2.c execution,  -O3 -fomit-frame-pointer 
FAIL: gcc.c-torture/execute/stdarg-2.c execution,  -O3 -g 
FAIL: gcc.c-torture/execute/stdarg-2.c execution,  -Os 
FAIL: gcc.c-torture/execute/stdarg-2.c 
execution,  -O2 -flto -flto-partition=none 
FAIL: gcc.c-torture/execute/stdarg-2.c execution,  -O2 -flto 

for the exact same reason.

Patch

diff --git a/gcc/df-problems.c b/gcc/df-problems.c
index 8928454..f9b0bc1 100644
--- a/gcc/df-problems.c
+++ b/gcc/df-problems.c
@@ -2748,10 +2748,12 @@  df_ignore_stack_reg (int regno ATTRIBUTE_UNUSED)
 
 
 /* Remove all of the REG_DEAD or REG_UNUSED notes from INSN and add
-   them to OLD_DEAD_NOTES and OLD_UNUSED_NOTES.  */
+   them to OLD_DEAD_NOTES and OLD_UNUSED_NOTES.  Remove also
+   REG_EQUAL/REG_EQUIV notes referring to dead pseudos using LIVE
+   as the bitmap of currently live registers.  */
 
 static void
-df_kill_notes (rtx insn)
+df_kill_notes (rtx insn, bitmap live)
 {
   rtx *pprev = &REG_NOTES (insn);
   rtx link = *pprev;
@@ -2798,6 +2800,45 @@  df_kill_notes (rtx insn)
 	    }
 	  break;
 
+	case REG_EQUAL:
+	case REG_EQUIV:
+	  {
+	    /* Remove the notes that refer to dead registers.  As we have at most
+	       one REG_EQUAL/EQUIV note, all of EQ_USES will refer to this note
+	       so we need to purge the complete EQ_USES vector when removing
+	       the note using df_notes_rescan.  */
+	    df_ref *use_rec;
+	    bool deleted = false;
+
+	    for (use_rec = DF_INSN_EQ_USES (insn); *use_rec; use_rec++)
+	      {
+		df_ref use = *use_rec;
+		if (DF_REF_REGNO (use) > FIRST_PSEUDO_REGISTER
+		    && (DF_REF_FLAGS (use) & DF_REF_IN_NOTE)
+		    && ! bitmap_bit_p (live, DF_REF_REGNO (use)))
+		  {
+		    deleted = true;
+		    break;
+		  }
+	      }
+	    if (deleted)
+	      {
+		rtx next;
+#ifdef REG_DEAD_DEBUGGING
+		df_print_note ("deleting: ", insn, link);
+#endif
+		next = XEXP (link, 1);
+		free_EXPR_LIST_node (link);
+		*pprev = link = next;
+		df_notes_rescan (insn);
+	      }
+	    else
+	      {
+		pprev = &XEXP (link, 1);
+		link = *pprev;
+	      }
+	    break;
+	  }
 	default:
 	  pprev = &XEXP (link, 1);
 	  link = *pprev;
@@ -3299,7 +3340,7 @@  df_note_bb_compute (unsigned int bb_index,
       debug_insn = DEBUG_INSN_P (insn);
 
       bitmap_clear (do_not_gen);
-      df_kill_notes (insn);
+      df_kill_notes (insn, live);
 
       /* Process the defs.  */
       if (CALL_P (insn))
diff --git a/gcc/testsuite/gcc.dg/pr51505.c b/gcc/testsuite/gcc.dg/pr51505.c
new file mode 100644
index 0000000..dbcd322
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/pr51505.c
@@ -0,0 +1,19 @@ 
+/* PR rtl-optimization/51505 */
+/* { dg-do compile } */
+/* { dg-options "-O --param max-cse-insns=1" } */
+struct S
+{
+char a[256];
+};
+
+int bar(struct S, char[16]);
+
+void foo ()
+{
+  struct S u, s1, s2;
+  char e[256];
+  char i;
+  e[i] = ~s1.a[i] & s2.a[i];
+  if (bar(u, e))
+    __builtin_abort ();
+}