Patchwork patch to fix PR55154

login
register
mail settings
Submitter Vladimir Makarov
Date Nov. 9, 2012, 3:35 p.m.
Message ID <509D22DF.8000306@redhat.com>
Download mbox | patch
Permalink /patch/198107/
State New
Headers show

Comments

Vladimir Makarov - Nov. 9, 2012, 3:35 p.m.
The following patch fixes

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

   The insn in question has 2 alternatives.  The first alternative is 
rejected because operand constraint is class containing non-allocated 
hard reg for given options.  The second alternative was rejected by LRA 
as the maximal cost was achieved. Reload sets up to close to maximal 
cost when it sees ! and this is the last operand (imho setting to max 
reject in reload is wrong as if ! were in the 1st operand, reload would 
also reject the alternative).  LRA accumulates the costs and with 
reloads of previous operands it easily achieved maximal cost.  The patch 
affects a sensible code of LRA and therefore was rigorously tested.

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

   Committed as rev. 193364.

2012-11-09  Vladimir Makarov  <vmakarov@redhat.com>

         PR tree-optimization/55154
         * lra-int.h (LRA_LOSER_COST_FACTOR, LRA_MAX_REJECT): New macros.
         * lra.c (setup_operand_alternative): Use them.
         * lra-constraints.c (LOSER_COST_FACTOR, MAX_OVERALL_COST_BOUND):
         Remove.
         (process_alt_operands): Use LRA_LOSER_COST_FACTOR and
         LRA_MAX_REJECT.  Accumulate reject instead of setting for
         non-const.
         (curr_insn_transform): Initialize best_losers and best_overall by
         INT_MAX.

2012-11-09  Vladimir Makarov <vmakarov@redhat.com>

         PR rtl-optimization/55154
         * gcc.target/i386/pr55154.c: New test.

Patch

Index: lra.c
===================================================================
--- lra.c	(revision 193349)
+++ lra.c	(working copy)
@@ -784,10 +784,10 @@  setup_operand_alternative (lra_insn_reco
 		  break;
 
 		case '?':
-		  op_alt->reject += 6;
+		  op_alt->reject += LRA_LOSER_COST_FACTOR;
 		  break;
 		case '!':
-		  op_alt->reject += 600;
+		  op_alt->reject += LRA_MAX_REJECT;
 		  break;
 		case '&':
 		  op_alt->earlyclobber = 1;
Index: lra-constraints.c
===================================================================
--- lra-constraints.c	(revision 193349)
+++ lra-constraints.c	(working copy)
@@ -1282,12 +1282,6 @@  general_constant_p (rtx x)
   return CONSTANT_P (x) && (! flag_pic || LEGITIMATE_PIC_OPERAND_P (x));
 }
 
-/* Cost factor for each additional reload and maximal cost bound for
-   insn reloads.  One might ask about such strange numbers.  Their
-   values occurred historically from former reload pass.  */
-#define LOSER_COST_FACTOR 6
-#define MAX_OVERALL_COST_BOUND 600
-
 /* Major function to choose the current insn alternative and what
    operands should be reloaded and how.	 If ONLY_ALTERNATIVE is not
    negative we should consider only this alternative.  Return false if
@@ -1576,6 +1570,7 @@  process_alt_operands (int only_alternati
 		    badop = false;
 		    this_alternative = curr_alt[m];
 		    COPY_HARD_REG_SET (this_alternative_set, curr_alt_set[m]);
+		    winreg = this_alternative != NO_REGS;
 		    break;
 		  }
 
@@ -1828,7 +1823,7 @@  process_alt_operands (int only_alternati
 		     might cost something but probably less than old
 		     reload pass believes.  */
 		  if (lra_former_scratch_p (REGNO (operand_reg[nop])))
-		    reject += LOSER_COST_FACTOR;
+		    reject += LRA_LOSER_COST_FACTOR;
 		}
 	    }
 	  else if (did_match)
@@ -1912,20 +1907,15 @@  process_alt_operands (int only_alternati
 		      && no_input_reloads_p && ! const_to_mem))
 		goto fail;
 
-	      /* If we can't reload this value at all, reject this
-		 alternative.  Note that we could also lose due to
-		 LIMIT_RELOAD_CLASS, but we don't check that here.  */
-	      if (! CONSTANT_P (op) && ! no_regs_p)
-		{
-		  if (targetm.preferred_reload_class
-		      (op, this_alternative) == NO_REGS)
-		    reject = MAX_OVERALL_COST_BOUND;
-
-		  if (curr_static_id->operand[nop].type == OP_OUT
-		      && (targetm.preferred_output_reload_class
-			  (op, this_alternative) == NO_REGS))
-		    reject = MAX_OVERALL_COST_BOUND;
-		}
+	      /* Check strong discouragement of reload of non-constant
+		 into class THIS_ALTERNATIVE.  */
+	      if (! CONSTANT_P (op) && ! no_regs_p
+		  && (targetm.preferred_reload_class
+		      (op, this_alternative) == NO_REGS
+		      || (curr_static_id->operand[nop].type == OP_OUT
+			  && (targetm.preferred_output_reload_class
+			      (op, this_alternative) == NO_REGS))))
+		reject += LRA_MAX_REJECT;
 
 	      if (! ((const_to_mem && constmemok)
 		     || (MEM_P (op) && offmemok)))
@@ -1966,7 +1956,7 @@  process_alt_operands (int only_alternati
 	     Should we update the cost (may be approximately) here
 	     because of early clobber register reloads or it is a rare
 	     or non-important thing to be worth to do it.  */
-	  overall = losers * LOSER_COST_FACTOR + reject;
+	  overall = losers * LRA_LOSER_COST_FACTOR + reject;
 	  if ((best_losers == 0 || losers != 0) && best_overall < overall)
 	    goto fail;
 
@@ -2019,7 +2009,7 @@  process_alt_operands (int only_alternati
 	      {
 		curr_alt_match_win[j] = false;
 		losers++;
-		overall += LOSER_COST_FACTOR;
+		overall += LRA_LOSER_COST_FACTOR;
 	      }
 	  if (! curr_alt_match_win[i])
 	    curr_alt_dont_inherit_ops[curr_alt_dont_inherit_ops_num++] = i;
@@ -2032,7 +2022,7 @@  process_alt_operands (int only_alternati
 	    }
 	  curr_alt_win[i] = curr_alt_match_win[i] = false;
 	  losers++;
-	  overall += LOSER_COST_FACTOR;
+	  overall += LRA_LOSER_COST_FACTOR;
 	}
       small_class_operands_num = 0;
       for (nop = 0; nop < n_operands; nop++)
@@ -2635,7 +2625,7 @@  curr_insn_transform (void)
      the wrong kind of hard reg.  For this, we must consider all the
      operands together against the register constraints.  */
 
-  best_losers = best_overall = MAX_RECOG_OPERANDS * 2 + MAX_OVERALL_COST_BOUND;
+  best_losers = best_overall = INT_MAX;
   best_small_class_operands_num = best_reload_sum = 0;
 
   curr_swapped = false;
Index: lra-int.h
===================================================================
--- lra-int.h	(revision 193349)
+++ lra-int.h	(working copy)
@@ -243,6 +243,12 @@  typedef struct lra_insn_recog_data *lra_
 #define LRA_TEMP_CLOBBER_P(x) \
   (RTL_FLAG_CHECK1 ("TEMP_CLOBBER_P", (x), CLOBBER)->unchanging)
 
+/* Cost factor for each additional reload and maximal cost reject for
+   insn reloads.  One might ask about such strange numbers.  Their
+   values occurred historically from former reload pass.  */
+#define LRA_LOSER_COST_FACTOR 6
+#define LRA_MAX_REJECT 600
+
 /* lra.c: */
 
 extern FILE *lra_dump_file;
Index: testsuite/gcc.target/i386/pr55154.c
===================================================================
--- testsuite/gcc.target/i386/pr55154.c	(revision 0)
+++ testsuite/gcc.target/i386/pr55154.c	(working copy)
@@ -0,0 +1,11 @@ 
+/* { dg-do compile } */
+/* { dg-require-effective-target lp64 } */
+/* { dg-require-effective-target fpic } */
+/* { dg-options "-O2 -mcx16 -fpic -mcmodel=large -fno-split-wide-types" } */
+
+__int128 i;
+
+void test ()
+{
+  __sync_val_compare_and_swap (&i, i, i);
+}