Patchwork [v3,i386] : Fix PR46829 and PR46843, ICE in spill_failure with -fschedule-insns on x86

login
register
mail settings
Submitter Uros Bizjak
Date Aug. 23, 2012, 6:04 p.m.
Message ID <CAFULd4Yu3UdbSN9OFHsnUBF5q8X7dkryWJu9kN-8UBaRL-og8w@mail.gmail.com>
Download mbox | patch
Permalink /patch/179700/
State New
Headers show

Comments

Uros Bizjak - Aug. 23, 2012, 6:04 p.m.
Hello!

Attached patch implements TARGET_COMBINE_REJECT_INSN target hook [1] for x86.

The patched gcc rejects combined instructions with hard registers that
don't fit operand constraints, and this way helps reload to avoid
spill failures. Please note that critical instructions include
instructions like integer division, multiplication, variable shifts
and various string instructions that already require eax, edx and/or
ecx registers at exactly the right spot. Having this valuable register
at the wrong place could trigger the spill failure, since reload is
unable to solve the mess.

The situation is even more critical with insn scheduling, since
various moves to and from critical registers get scheduled before or
after critical instruction, increasing register pressure. A follow-up
patch from Yuri will enable priority scheduling for hard reg moves and
this will hopefully allow us to enable scheduling for various x86
targets.

2012-08-23  Uros Bizjak  <ubizjak@gmail.com>

	PR target/46829
	PR target/46843
	* config/i386/i386.c (ix86_reject_combined_insn): New function.
	(TARGET_REJECT_COMBINED_INSN): New macro.

testsuite/ChangeLog:

2012-08-23  Uros Bizjak  <ubizjak@gmail.com>

	PR target/46829
	PR target/46843
	* gcc.target/i386/pr46829.c: New test.
	* gcc.target/i386/pr46843.c: New test.

Patch was bootstrapped and regression tested on x86_64-pc-linux-gnu {,-m32}.

I will wait a couple of days with the patch, so eventual effects on
benchmarks can be analysed.

[1] http://gcc.gnu.org/ml/gcc-patches/2012-08/msg01594.html

Uros.

Patch

Index: config/i386/i386.c
===================================================================
--- config/i386/i386.c	(revision 190500)
+++ config/i386/i386.c	(working copy)
@@ -5539,6 +5539,77 @@  ix86_return_pops_args (tree fundecl, tree funtype,
 
   return 0;
 }
+
+/* Implement the TARGET_REJECT_COMBINED_INSN hook.  */
+
+static bool
+ix86_reject_combined_insn (rtx insn)
+{
+  /* Check operand constraints in case hard registers were propagated
+     into insn pattern.  This check prevents combine pass from
+     generating insn patterns with invalid hard register operands.
+     These invalid insns can eventually confuse reload to error out
+     with a spill failure.  See also PRs 46829 and 46843.  */
+  if ((INSN_CODE (insn) = recog (PATTERN (insn), insn, 0)) >= 0)
+    {
+      int i;
+
+      extract_insn (insn);
+      preprocess_constraints ();
+
+      for (i = 0; i < recog_data.n_operands; i++)
+	{
+	  rtx op = recog_data.operand[i];
+	  enum machine_mode mode = GET_MODE (op);
+	  struct operand_alternative *op_alt;
+	  int offset = 0;
+	  bool win;
+	  int j;
+
+	  /* A unary operator may be accepted by the predicate, but it
+	     is irrelevant for matching constraints.  */
+	  if (UNARY_P (op))
+	    op = XEXP (op, 0);
+
+	  if (GET_CODE (op) == SUBREG)
+	    {
+	      if (REG_P (SUBREG_REG (op))
+		  && REGNO (SUBREG_REG (op)) < FIRST_PSEUDO_REGISTER)
+		offset = subreg_regno_offset (REGNO (SUBREG_REG (op)),
+					      GET_MODE (SUBREG_REG (op)),
+					      SUBREG_BYTE (op),
+					      GET_MODE (op));
+	      op = SUBREG_REG (op);
+	    }
+
+	  if (!(REG_P (op) && HARD_REGISTER_P (op)))
+	    continue;
+
+	  op_alt = recog_op_alt[i];
+
+	  /* Operand has no constraints, anything is OK.  */
+ 	  win = !recog_data.n_alternatives;
+
+	  for (j = 0; j < recog_data.n_alternatives; j++)
+	    {
+	      if (op_alt[j].anything_ok
+		  || (op_alt[j].matches != -1
+		      && rtx_equal_p (recog_data.operand[i],
+				      recog_data.operand[op_alt[j].matches]))
+		  || reg_fits_class_p (op, op_alt[j].cl, offset, mode))
+		{
+		  win = true;
+		  break;
+		}
+	    }
+
+	  if (!win)
+	    return true;
+	}
+    }
+
+  return false;
+}
 
 /* Argument support functions.  */
 
@@ -40697,6 +40768,8 @@  ix86_memmodel_check (unsigned HOST_WIDE_INT val)
 #define TARGET_TRAMPOLINE_INIT ix86_trampoline_init
 #undef TARGET_RETURN_POPS_ARGS
 #define TARGET_RETURN_POPS_ARGS ix86_return_pops_args
+#undef TARGET_REJECT_COMBINED_INSN
+#define TARGET_REJECT_COMBINED_INSN ix86_reject_combined_insn
 
 #undef TARGET_GIMPLIFY_VA_ARG_EXPR
 #define TARGET_GIMPLIFY_VA_ARG_EXPR ix86_gimplify_va_arg
Index: testsuite/gcc.target/i386/pr46843.c
===================================================================
--- testsuite/gcc.target/i386/pr46843.c	(revision 0)
+++ testsuite/gcc.target/i386/pr46843.c	(working copy)
@@ -0,0 +1,12 @@ 
+/* { dg-do compile } */
+/* { dg-options "-O2 -fschedule-insns" } */
+
+void foo (double *d1, double *u1, double *u2, double *d2, int s, int j, int i)
+{
+  int n = 1 << s;
+  double x = 0;
+
+  for (; j < n; j++)
+    x += d1[j] * d2[i];
+  d1[i] = x;
+}
Index: testsuite/gcc.target/i386/pr46829.c
===================================================================
--- testsuite/gcc.target/i386/pr46829.c	(revision 0)
+++ testsuite/gcc.target/i386/pr46829.c	(working copy)
@@ -0,0 +1,18 @@ 
+/* { dg-do compile } */
+/* { dg-options "-O2 -fschedule-insns" } */
+
+struct S
+{
+  int i, j;
+};
+
+extern struct S s[];
+
+extern void bar (int, ...);
+
+void
+foo (int n)
+{
+  while (s[n].i)
+    bar (0, n, s[n].j, s, s[n].i / s[n].j);
+}