Patchwork A fix for PR40386

login
register
mail settings
Submitter Vladimir Makarov
Date Sept. 8, 2010, 7:21 p.m.
Message ID <4C87E254.7080209@redhat.com>
Download mbox | patch
Permalink /patch/64199/
State New
Headers show

Comments

Vladimir Makarov - Sept. 8, 2010, 7:21 p.m.
The following patch fixes PR40386.  The problem in details is 
described on http://gcc.gnu.org/bugzilla/show_bug.cgi?id=40386.

Is it ok to commit into trunk and gcc-4.4-branch?

2010-09-08  Vladimir Makarov <vmakarov@redhat.com>

         PR middle-end/40386
         * ira.c (pseudo_for_reload_consideration_p): Don't use
         flag_ira_share_spill_slots.

2010-09-08  Vladimir Makarov <vmakarov@redhat.com>

         PR middle-end/40386
         * gcc.c-torture/execute/{pr40386.c,pr40386.x}: New testcase.
H.J. Lu - Sept. 8, 2010, 7:26 p.m.
On Wed, Sep 8, 2010 at 12:21 PM, Vladimir Makarov <vmakarov@redhat.com> wrote:
>  The following patch fixes PR40386.  The problem in details is described on
> http://gcc.gnu.org/bugzilla/show_bug.cgi?id=40386.
>
> Is it ok to commit into trunk and gcc-4.4-branch?

No 4.5?

> 2010-09-08  Vladimir Makarov <vmakarov@redhat.com>
>
>        PR middle-end/40386
>        * ira.c (pseudo_for_reload_consideration_p): Don't use
>        flag_ira_share_spill_slots.
>
> 2010-09-08  Vladimir Makarov <vmakarov@redhat.com>
>
>        PR middle-end/40386
>        * gcc.c-torture/execute/{pr40386.c,pr40386.x}: New testcase.
>
>
>
Vladimir Makarov - Sept. 8, 2010, 7:32 p.m.
On 09/08/2010 03:26 PM, H.J. Lu wrote:
> On Wed, Sep 8, 2010 at 12:21 PM, Vladimir Makarov<vmakarov@redhat.com>  wrote:
>>   The following patch fixes PR40386.  The problem in details is described on
>> http://gcc.gnu.org/bugzilla/show_bug.cgi?id=40386.
>>
>> Is it ok to commit into trunk and gcc-4.4-branch?
> No 4.5?
>
>
Should be also 4.5.  Thanks for the correction, H.J.
Bernd Schmidt - Sept. 8, 2010, 8:20 p.m.
On 09/08/2010 09:21 PM, Vladimir Makarov wrote:
>  The following patch fixes PR40386.  The problem in details is described
> on http://gcc.gnu.org/bugzilla/show_bug.cgi?id=40386.

According to that,

> The problem is in that pseudos (r121 in our case) spilled by IRA are
> not added to live_throughout of reload chain.

Is there a good reason to do this, or should we change that instead?


Bernd
Vladimir Makarov - Sept. 8, 2010, 9:05 p.m.
On 09/08/2010 04:20 PM, Bernd Schmidt wrote:
> On 09/08/2010 09:21 PM, Vladimir Makarov wrote:
>>   The following patch fixes PR40386.  The problem in details is described
>> on http://gcc.gnu.org/bugzilla/show_bug.cgi?id=40386.
> According to that,
>
>> The problem is in that pseudos (r121 in our case) spilled by IRA are
>> not added to live_throughout of reload chain.
> Is there a good reason to do this, or should we change that instead?
>
We should add all pseudos (I guess it was my typo).  The patch actually 
results in adding all pseudos (spilled or not) to live_throughout 
independently from value of flag_ira_share_spill_slots.  For the old RA 
it was done only for non-spilled pseudos.  IRA needs this because it can 
assign hard registers in reload to pseudos spilled by IRA which global.c 
never did.
Bernd Schmidt - Sept. 8, 2010, 9:10 p.m.
On 09/08/2010 11:05 PM, Vladimir Makarov wrote:
>  On 09/08/2010 04:20 PM, Bernd Schmidt wrote:
>> On 09/08/2010 09:21 PM, Vladimir Makarov wrote:
>>>   The following patch fixes PR40386.  The problem in details is
>>> described
>>> on http://gcc.gnu.org/bugzilla/show_bug.cgi?id=40386.
>> According to that,
>>
>>> The problem is in that pseudos (r121 in our case) spilled by IRA are
>>> not added to live_throughout of reload chain.
>> Is there a good reason to do this, or should we change that instead?
>>
> We should add all pseudos (I guess it was my typo).  The patch actually
> results in adding all pseudos (spilled or not) to live_throughout
> independently from value of flag_ira_share_spill_slots.  For the old RA
> it was done only for non-spilled pseudos.  IRA needs this because it can
> assign hard registers in reload to pseudos spilled by IRA which global.c
> never did.

Ok, that makes sense.  Please install the patch.


Bernd
Mikael Pettersson - Sept. 12, 2010, 10:32 a.m.
Vladimir Makarov writes:
 >   The following patch fixes PR40386.  The problem in details is 
 > described on http://gcc.gnu.org/bugzilla/show_bug.cgi?id=40386.
 > 
 > Is it ok to commit into trunk and gcc-4.4-branch?
 > 
 > 2010-09-08  Vladimir Makarov <vmakarov@redhat.com>
 > 
 >          PR middle-end/40386
 >          * ira.c (pseudo_for_reload_consideration_p): Don't use
 >          flag_ira_share_spill_slots.
 > 
 > 2010-09-08  Vladimir Makarov <vmakarov@redhat.com>
 > 
 >          PR middle-end/40386
 >          * gcc.c-torture/execute/{pr40386.c,pr40386.x}: New testcase.

The version checked into gcc-4_4-branch has a broken test case
which fails to compile due to redefinition errors.  It looks like
a patch to add the test case was applied twice, causing the files
to contain duplicated text.  Here's is what 4.4's pr40386.x looks
like right now:

> set additional_flags "-fno-ira-share-spill-slots"
> return 0
> set additional_flags "-fno-ira-share-spill-slots"
> return 0

pr40386.c has the same issue, which causes it to fail to compile.

/Mikael

Patch

Index: ira.c
===================================================================
--- ira.c	(revision 164011)
+++ ira.c	(working copy)
@@ -2742,8 +2742,7 @@  pseudo_for_reload_consideration_p (int r
 {
   /* Consider spilled pseudos too for IRA because they still have a
      chance to get hard-registers in the reload when IRA is used.  */
-  return (reg_renumber[regno] >= 0
-	  || (ira_conflicts_p && flag_ira_share_spill_slots));
+  return (reg_renumber[regno] >= 0 || ira_conflicts_p);
 }
 
 /* Init LIVE_SUBREGS[ALLOCNUM] and LIVE_SUBREGS_USED[ALLOCNUM] using
Index: testsuite/gcc.c-torture/execute/pr40386.c
===================================================================
--- testsuite/gcc.c-torture/execute/pr40386.c	(revision 0)
+++ testsuite/gcc.c-torture/execute/pr40386.c	(revision 0)
@@ -0,0 +1,99 @@ 
+/* { dg-options "-fno-ira-share-spill-slots" } */
+#define CHAR_BIT 8
+
+#define ROR(a,b) (((a) >> (b)) | ((a) << ((sizeof (a) * CHAR_BIT) - (b))))
+#define ROL(a,b) (((a) << (b)) | ((a) >> ((sizeof (a) * CHAR_BIT) - (b))))
+
+#define CHAR_VALUE ((char)0xf234)
+#define SHORT_VALUE ((short)0xf234)
+#define INT_VALUE ((int)0xf234)
+#define LONG_VALUE ((long)0xf2345678L)
+#define LL_VALUE ((long long)0xf2345678abcdef0LL)
+
+#define SHIFT1 4
+#define SHIFT2 ((sizeof (long long) * CHAR_BIT) - SHIFT1)
+
+char c = CHAR_VALUE;
+short s = SHORT_VALUE;
+int i = INT_VALUE;
+long l = LONG_VALUE;
+long long ll = LL_VALUE;
+int shift1 = SHIFT1;
+int shift2 = SHIFT2;
+
+main ()
+{
+  if (ROR (c, shift1) != ROR (CHAR_VALUE, SHIFT1))
+    abort ();
+
+  if (ROR (c, SHIFT1) != ROR (CHAR_VALUE, SHIFT1))
+    abort ();
+
+  if (ROR (s, shift1) != ROR (SHORT_VALUE, SHIFT1))
+    abort ();
+
+  if (ROR (s, SHIFT1) != ROR (SHORT_VALUE, SHIFT1))
+    abort ();
+
+  if (ROR (i, shift1) != ROR (INT_VALUE, SHIFT1))
+    abort ();
+
+  if (ROR (i, SHIFT1) != ROR (INT_VALUE, SHIFT1))
+    abort ();
+
+  if (ROR (l, shift1) != ROR (LONG_VALUE, SHIFT1))
+    abort ();
+
+  if (ROR (l, SHIFT1) != ROR (LONG_VALUE, SHIFT1))
+    abort ();
+
+  if (ROR (ll, shift1) != ROR (LL_VALUE, SHIFT1))
+    abort ();
+
+  if (ROR (ll, SHIFT1) != ROR (LL_VALUE, SHIFT1))
+    abort ();
+
+  if (ROR (ll, shift2) != ROR (LL_VALUE, SHIFT2))
+    abort ();
+
+  if (ROR (ll, SHIFT2) != ROR (LL_VALUE, SHIFT2))
+    abort ();
+
+  if (ROL (c, shift1) != ROL (CHAR_VALUE, SHIFT1))
+    abort ();
+
+  if (ROL (c, SHIFT1) != ROL (CHAR_VALUE, SHIFT1))
+    abort ();
+
+  if (ROL (s, shift1) != ROL (SHORT_VALUE, SHIFT1))
+    abort ();
+
+  if (ROL (s, SHIFT1) != ROL (SHORT_VALUE, SHIFT1))
+    abort ();
+
+  if (ROL (i, shift1) != ROL (INT_VALUE, SHIFT1))
+    abort ();
+
+  if (ROL (i, SHIFT1) != ROL (INT_VALUE, SHIFT1))
+    abort ();
+
+  if (ROL (l, shift1) != ROL (LONG_VALUE, SHIFT1))
+    abort ();
+
+  if (ROL (l, SHIFT1) != ROL (LONG_VALUE, SHIFT1))
+    abort ();
+
+  if (ROL (ll, shift1) != ROL (LL_VALUE, SHIFT1))
+    abort ();
+
+  if (ROL (ll, SHIFT1) != ROL (LL_VALUE, SHIFT1))
+    abort ();
+
+  if (ROL (ll, shift2) != ROL (LL_VALUE, SHIFT2))
+    abort ();
+
+  if (ROL (ll, SHIFT2) != ROL (LL_VALUE, SHIFT2))
+    abort ();
+
+  exit (0);
+}
Index: testsuite/gcc.c-torture/execute/pr40386.x
===================================================================
--- testsuite/gcc.c-torture/execute/pr40386.x	(revision 0)
+++ testsuite/gcc.c-torture/execute/pr40386.x	(revision 0)
@@ -0,0 +1,2 @@ 
+set additional_flags "-fno-ira-share-spill-slots"
+return 0