[PATCH, rtl-optimization]: Fix PR46829, ICE in spill_failure with -fschedule-insns [was: Fixing instability of -fschedule-insns for x86]

[Uros Bizjak]

Hello!

Attached patch fixes one of the critical problems in combine.c:
combine pass blindly propagates hard registers into insn patterns and
this way creates partially invalid instructions. Most of the times,
reload is able to fix these inconsistencies, but operands with invalid
hard registers block IRA/reload to allocate most appropriate registers
from critically limited register sets, leading to spill failures.

To counter these problems, x86 port invented operand predicates like
"reg_not_xmm0_operand" and derivatives that prevented combine pass
from propagating invalid hard regs. This approach in fact papered over
real problem in the gcc infrastructure.

Attached patch introduces operand constraint checks directly into
recog_for_combine. Operands of valid instructions are checked against
their constraints and combined insn is rejected if they doesn't fit.

IMO, this patch is prerequisite for enabling scheduling pass on x86.
Proposed priority scheduling of function operands patch will hopefully
fix other issues with limited register set hard register allocations.

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

	PR rtl-optimization/46829
	* combine.c (recog_for_combine): Check operand constraints
	in case hard registers were propagater into insn pattern.

testsuite/ChangeLog:

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

	PR rtl-optimization/46829
	* gcc.target/i386/pr46829.c: New test.

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

OK for mainline?

Uros.
-------------- next part --------------
Index: combine.c
===================================================================
--- combine.c	(revision 190480)
+++ combine.c	(working copy)
@@ -10507,6 +10507,7 @@ recog_for_combine (rtx *pnewpat, rtx insn, rtx *pn
   int i;
   rtx notes = 0;
   rtx old_notes, old_pat;
+  int old_icode;
 
   /* If PAT is a PARALLEL, check to see if it contains the CLOBBER
      we use to indicate that something didn't match.  If we find such a
@@ -10566,6 +10567,7 @@ recog_for_combine (rtx *pnewpat, rtx insn, rtx *pn
 	  print_rtl_single (dump_file, pat);
 	}
     }
+
   PATTERN (insn) = old_pat;
   REG_NOTES (insn) = old_notes;
 
@@ -10607,6 +10609,86 @@ recog_for_combine (rtx *pnewpat, rtx insn, rtx *pn
       pat = newpat;
     }
 
+  old_pat = PATTERN (insn);
+  old_notes = REG_NOTES (insn);
+  old_icode = INSN_CODE (insn);
+  PATTERN (insn) = pat;
+  REG_NOTES (insn) = notes;
+
+  /* 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 PR 46829.  */
+  if (insn_code_number >= 0
+      && insn_code_number != NOOP_MOVE_INSN_CODE
+      && (INSN_CODE (insn) = recog (PATTERN (insn), insn, 0)) >= 0)
+    {
+      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];
+
+	  win = false;
+	  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[j],
+				      recog_data.operand[op_alt[j].matches]))
+		  || (reg_fits_class_p (op, op_alt[j].cl, offset, mode)))
+		{
+		  win = true;
+		  break;
+		}
+	    }
+
+	  if (!win)
+	    {
+	      if (dump_file && (dump_flags & TDF_DETAILS))
+		{
+		  fputs ("Operand failed to match constraints:\n",
+			 dump_file);
+		  print_rtl_single (dump_file, op);
+		}
+	      insn_code_number = -1;
+	      break;
+	    }
+	}
+    }
+
+  PATTERN (insn) = old_pat;
+  REG_NOTES (insn) = old_notes;
+  INSN_CODE (insn) = old_icode;
+
   *pnewpat = pat;
   *pnotes = notes;
 
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);
+}