diff mbox

patch to fix PR55775

Message ID 50D4D307.109@redhat.com
State New
Headers show

Commit Message

Vladimir Makarov Dec. 21, 2012, 9:22 p.m. UTC
The following patch fixes

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=55775

The bug is related to PR55330.

The patch was successfully bootstrapped and tested on x86/x86-64.

Committed as rev. 194680.

2012-12-21  Vladimir Makarov  <vmakarov@redhat.com>

         PR middle-end/55775
         * lra-assigns.c (improve_inheritance): Do nothing after
         LRA_MAX_INHERITANCE_PASSES pass.
         * lra-constraints.c (MAX_CONSTRAINT_ITERATION_NUMBER): Rename to
         LRA_MAX_CONSTRAINT_ITERATION_NUMBER.  Move to lra-int.h.
         (MAX_INHERITANCE_PASSES): Rename to LRA_MAX_INHERITANCE_PASSES.
         Move to lra-int.h.
         * lra-int.h (LRA_MAX_CONSTRAINT_ITERATION_NUMBER): Move from
         lra-constraints.c.
         (LRA_MAX_INHERITANCE_PASSES): Ditto.

2012-12-21  Vladimir Makarov  <vmakarov@redhat.com>

         PR middle-end/55775
         * gcc.target/i386/pr55775.c: New test.

Comments

David Miller Dec. 21, 2012, 9:29 p.m. UTC | #1
From: Vladimir Makarov <vmakarov@redhat.com>
Date: Fri, 21 Dec 2012 16:22:15 -0500

> 2012-12-21  Vladimir Makarov  <vmakarov@redhat.com>
> 
>         PR middle-end/55775
>         * lra-assigns.c (improve_inheritance): Do nothing after
>         LRA_MAX_INHERITANCE_PASSES pass.
>         * lra-constraints.c (MAX_CONSTRAINT_ITERATION_NUMBER): Rename to
>         LRA_MAX_CONSTRAINT_ITERATION_NUMBER.  Move to lra-int.h.
>         (MAX_INHERITANCE_PASSES): Rename to LRA_MAX_INHERITANCE_PASSES.
>         Move to lra-int.h.
>         * lra-int.h (LRA_MAX_CONSTRAINT_ITERATION_NUMBER): Move from
>         lra-constraints.c.
>         (LRA_MAX_INHERITANCE_PASSES): Ditto.

The changes to lra_inheritabnce() and lra_undo_inheritance() are not
listed.

Also, why is it OK to simply ignore the fact that a relaxation
algorithm is still making changes?  Shouldn't we be instead fixing
whatever prevents proper convergence?
Vladimir Makarov Dec. 21, 2012, 10:04 p.m. UTC | #2
On 12-12-21 4:29 PM, David Miller wrote:
> From: Vladimir Makarov <vmakarov@redhat.com>
> Date: Fri, 21 Dec 2012 16:22:15 -0500
>
>> 2012-12-21  Vladimir Makarov  <vmakarov@redhat.com>
>>
>>          PR middle-end/55775
>>          * lra-assigns.c (improve_inheritance): Do nothing after
>>          LRA_MAX_INHERITANCE_PASSES pass.
>>          * lra-constraints.c (MAX_CONSTRAINT_ITERATION_NUMBER): Rename to
>>          LRA_MAX_CONSTRAINT_ITERATION_NUMBER.  Move to lra-int.h.
>>          (MAX_INHERITANCE_PASSES): Rename to LRA_MAX_INHERITANCE_PASSES.
>>          Move to lra-int.h.
>>          * lra-int.h (LRA_MAX_CONSTRAINT_ITERATION_NUMBER): Move from
>>          lra-constraints.c.
>>          (LRA_MAX_INHERITANCE_PASSES): Ditto.
> The changes to lra_inheritabnce() and lra_undo_inheritance() are not
> listed.
It is assumed, David.  I should have written "rename everywhere".
> Also, why is it OK to simply ignore the fact that a relaxation
> algorithm is still making changes?  Shouldn't we be instead fixing
> whatever prevents proper convergence?
It is very hard to fix according to your proposals.  It would require to 
insert most of code from lra-constraints.c because code for improving 
inheritance should take a lot of details about insn constraints into 
account to fix it.

If I go this way, it will be another reload which is trying to do 
everything at once.  Also after 2 passes the inheritance improve code 
(as inheritance code itself) usually does nothing for big majority of 
programs.  It has no sense to run them all the time.  So for stability, 
LRA after a few iterations is trying to do only most important and 
necessary code for generation of the correct code.
David Miller Dec. 22, 2012, 2:22 a.m. UTC | #3
From: Vladimir Makarov <vmakarov@redhat.com>
Date: Fri, 21 Dec 2012 17:04:34 -0500

> If I go this way, it will be another reload which is trying to do
> everything at once.  Also after 2 passes the inheritance improve code
> (as inheritance code itself) usually does nothing for big majority of
> programs.  It has no sense to run them all the time.  So for
> stability, LRA after a few iterations is trying to do only most
> important and necessary code for generation of the correct code.

Thanks for explaining.
diff mbox

Patch

Index: lra-assigns.c
===================================================================
--- lra-assigns.c	(revision 194677)
+++ lra-assigns.c	(working copy)
@@ -1084,6 +1084,8 @@  improve_inheritance (bitmap changed_pseu
   lra_copy_t cp, next_cp;
   bitmap_iterator bi;
 
+  if (lra_inheritance_iter > LRA_MAX_INHERITANCE_PASSES)
+    return;
   n = 0;
   EXECUTE_IF_SET_IN_BITMAP (&lra_inheritance_pseudos, 0, k, bi)
     if (reg_renumber[k] >= 0 && lra_reg_info[k].nrefs != 0)
Index: lra-constraints.c
===================================================================
--- lra-constraints.c	(revision 194677)
+++ lra-constraints.c	(working copy)
@@ -3201,10 +3201,6 @@  loc_equivalence_callback (rtx loc, const
   return NULL_RTX;
 }
 
-/* Maximum allowed number of constraint pass iterations after the last
-   spill pass.	It is for preventing LRA cycling in a bug case.	 */
-#define MAX_CONSTRAINT_ITERATION_NUMBER 30
-
 /* Maximum number of generated reload insns per an insn.  It is for
    preventing this pass cycling in a bug case.	*/
 #define MAX_RELOAD_INSNS_NUMBER LRA_MAX_INSN_RELOADS
@@ -3328,10 +3324,10 @@  lra_constraints (bool first_p)
     fprintf (lra_dump_file, "\n********** Local #%d: **********\n\n",
 	     lra_constraint_iter);
   lra_constraint_iter_after_spill++;
-  if (lra_constraint_iter_after_spill > MAX_CONSTRAINT_ITERATION_NUMBER)
+  if (lra_constraint_iter_after_spill > LRA_MAX_CONSTRAINT_ITERATION_NUMBER)
     internal_error
       ("Maximum number of LRA constraint passes is achieved (%d)\n",
-       MAX_CONSTRAINT_ITERATION_NUMBER);
+       LRA_MAX_CONSTRAINT_ITERATION_NUMBER);
   changed_p = false;
   lra_risky_transformations_p = false;
   new_insn_uid_start = get_max_uid ();
@@ -4698,21 +4694,6 @@  inherit_in_ebb (rtx head, rtx tail)
   return change_p;
 }
 
-/* The maximal number of inheritance/split passes in LRA.  It should
-   be more 1 in order to perform caller saves transformations and much
-   less MAX_CONSTRAINT_ITERATION_NUMBER to prevent LRA to do as many
-   as permitted constraint passes in some complicated cases.  The
-   first inheritance/split pass has a biggest impact on generated code
-   quality.  Each subsequent affects generated code in less degree.
-   For example, the 3rd pass does not change generated SPEC2000 code
-   at all on x86-64.  */
-#define MAX_INHERITANCE_PASSES 2
-
-#if MAX_INHERITANCE_PASSES <= 0 \
-    || MAX_INHERITANCE_PASSES >= MAX_CONSTRAINT_ITERATION_NUMBER - 8
-#error wrong MAX_INHERITANCE_PASSES value
-#endif
-
 /* This value affects EBB forming.  If probability of edge from EBB to
    a BB is not greater than the following value, we don't add the BB
    to EBB.  */
@@ -4730,7 +4711,7 @@  lra_inheritance (void)
   edge e;
 
   lra_inheritance_iter++;
-  if (lra_inheritance_iter > MAX_INHERITANCE_PASSES)
+  if (lra_inheritance_iter > LRA_MAX_INHERITANCE_PASSES)
     return;
   timevar_push (TV_LRA_INHERITANCE);
   if (lra_dump_file != NULL)
@@ -5000,7 +4981,7 @@  lra_undo_inheritance (void)
   bool change_p;
 
   lra_undo_inheritance_iter++;
-  if (lra_undo_inheritance_iter > MAX_INHERITANCE_PASSES)
+  if (lra_undo_inheritance_iter > LRA_MAX_INHERITANCE_PASSES)
     return false;
   if (lra_dump_file != NULL)
     fprintf (lra_dump_file,
Index: lra-int.h
===================================================================
--- lra-int.h	(revision 194677)
+++ lra-int.h	(working copy)
@@ -249,6 +249,25 @@  typedef struct lra_insn_recog_data *lra_
 #define LRA_LOSER_COST_FACTOR 6
 #define LRA_MAX_REJECT 600
 
+/* Maximum allowed number of constraint pass iterations after the last
+   spill pass.	It is for preventing LRA cycling in a bug case.	 */
+#define LRA_MAX_CONSTRAINT_ITERATION_NUMBER 30
+
+/* The maximal number of inheritance/split passes in LRA.  It should
+   be more 1 in order to perform caller saves transformations and much
+   less MAX_CONSTRAINT_ITERATION_NUMBER to prevent LRA to do as many
+   as permitted constraint passes in some complicated cases.  The
+   first inheritance/split pass has a biggest impact on generated code
+   quality.  Each subsequent affects generated code in less degree.
+   For example, the 3rd pass does not change generated SPEC2000 code
+   at all on x86-64.  */
+#define LRA_MAX_INHERITANCE_PASSES 2
+
+#if LRA_MAX_INHERITANCE_PASSES <= 0 \
+    || LRA_MAX_INHERITANCE_PASSES >= LRA_MAX_CONSTRAINT_ITERATION_NUMBER - 8
+#error wrong LRA_MAX_INHERITANCE_PASSES value
+#endif
+
 /* lra.c: */
 
 extern FILE *lra_dump_file;
Index: testsuite/gcc.target/i386/pr55775.c
===================================================================
--- testsuite/gcc.target/i386/pr55775.c	(revision 0)
+++ testsuite/gcc.target/i386/pr55775.c	(working copy)
@@ -0,0 +1,56 @@ 
+/* { dg-do compile } */
+/* { dg-options "-O1" } */
+
+int *ptr;
+int *fn1 (int *);
+int fn2 (int, int);
+int fn3 (void);
+int fn4 (int);
+
+static int
+foo (int x, int y, int z)
+{
+  int b;
+  asm ("" : "=a" (b), "=&d" (x) : "0" (y), "1" (x), "mr" (z));
+  return x;
+}
+
+static int
+bar (int x, int y)
+{
+  int a;
+  if (!y)
+    {
+      for (a = 0; a <= (x >> 1); )
+	;
+      a = foo (y, fn2 (2, x), x);
+      if (x)
+	a = x;
+      return a;
+    }
+}
+
+static int
+baz (int x, int y)
+{
+  int *a = ptr;
+  int t, xk1 = fn3 (), xk = x * xk1;
+  for (t = 0; t < xk; t += xk1)
+    {
+      if (fn4 (a[2]))
+	return -y;
+      a = fn1 (a);
+    }
+  return 0;
+}
+
+void
+test (int x, long y, int z)
+{
+  int a = fn3 ();
+  int b;
+  int c = bar (x, z);
+  for (b = 0; b <= y; b++)
+    c = baz (x, c);
+  fn2 (c, a);
+}