diff mbox

[3/5] Make recog_op_alt consumers check the enabled attribute

Message ID 87a99yfgx3.fsf_-_@talisman.default
State New
Headers show

Commit Message

Richard Sandiford May 31, 2014, 9:15 a.m. UTC 
As described in the covering note, it seems better to put the onus of
checking the enabled attribute on the passes that are walking each
alternative, like LRA does for its internal subpasses.  That leads
to a nicer interface in patch 4 and would make it easier to precompute
the information at build time.  (The only thing preventing that now is
the subunion class.)

Thanks,
Richard


gcc/
	* recog.c (preprocess_constraints): Don't skip disabled alternatives.
	* ira-lives.c (check_and_make_def_conflict): Check for disabled
	alternatives.
	(make_early_clobber_and_input_conflicts): Likewise.
	* config/i386/i386.c (ix86_legitimate_combined_insn): Likewise.

Comments

Jeff Law June 3, 2014, 9:58 p.m. UTC | #1 
On 05/31/14 03:15, Richard Sandiford wrote:
> As described in the covering note, it seems better to put the onus of
> checking the enabled attribute on the passes that are walking each
> alternative, like LRA does for its internal subpasses.  That leads
> to a nicer interface in patch 4 and would make it easier to precompute
> the information at build time.  (The only thing preventing that now is
> the subunion class.)
>
> Thanks,
> Richard
>
>
> gcc/
> 	* recog.c (preprocess_constraints): Don't skip disabled alternatives.
> 	* ira-lives.c (check_and_make_def_conflict): Check for disabled
> 	alternatives.
> 	(make_early_clobber_and_input_conflicts): Likewise.
> 	* config/i386/i386.c (ix86_legitimate_combined_insn): Likewise.
Did you spot check the other ports which utilized the enabled attribute 
to see if they need tweaking too?

I see aarch64, alpha, arc, arm, avr, c6x m68k, mips, mn10300, nds32, 
s390, sh & sparc.  I didn't check to see if any of them walk the 
alternatives in the backend.

Assuming you at least spot checked them, then this patch is OK.

jeff
Richard Sandiford June 4, 2014, 5:37 p.m. UTC | #2 
Thanks for the reviews, now committed

Jeff Law <law@redhat.com> writes:
> On 05/31/14 03:15, Richard Sandiford wrote:
>> As described in the covering note, it seems better to put the onus of
>> checking the enabled attribute on the passes that are walking each
>> alternative, like LRA does for its internal subpasses.  That leads
>> to a nicer interface in patch 4 and would make it easier to precompute
>> the information at build time.  (The only thing preventing that now is
>> the subunion class.)
>>
>> Thanks,
>> Richard
>>
>>
>> gcc/
>> 	* recog.c (preprocess_constraints): Don't skip disabled alternatives.
>> 	* ira-lives.c (check_and_make_def_conflict): Check for disabled
>> 	alternatives.
>> 	(make_early_clobber_and_input_conflicts): Likewise.
>> 	* config/i386/i386.c (ix86_legitimate_combined_insn): Likewise.
> Did you spot check the other ports which utilized the enabled attribute 
> to see if they need tweaking too?
>
> I see aarch64, alpha, arc, arm, avr, c6x m68k, mips, mn10300, nds32, 
> s390, sh & sparc.  I didn't check to see if any of them walk the 
> alternatives in the backend.

Yeah, the only port besides i386 to use preprocess_constraints is arm,
but it only looks at alternative which_alternative.

Richard
diff mbox

Patch

Index: gcc/recog.c
===================================================================
--- gcc/recog.c	2014-05-31 08:57:57.642085058 +0100
+++ gcc/recog.c	2014-05-31 09:07:21.669714878 +0100
@@ -2352,12 +2352,6 @@  preprocess_constraints (void)
 	  op_alt[i].matches = -1;
 	  op_alt[i].matched = -1;
 
-	  if (!TEST_BIT (recog_data.enabled_alternatives, j))
-	    {
-	      p = skip_alternative (p);
-	      continue;
-	    }
-
 	  if (*p == '\0' || *p == ',')
 	    {
 	      op_alt[i].anything_ok = 1;
Index: gcc/ira-lives.c
===================================================================
--- gcc/ira-lives.c	2014-05-31 08:54:12.238234755 +0100
+++ gcc/ira-lives.c	2014-05-31 09:07:21.670714886 +0100
@@ -641,8 +641,11 @@  check_and_make_def_conflict (int alt, in
       /* If there's any alternative that allows USE to match DEF, do not
 	 record a conflict.  If that causes us to create an invalid
 	 instruction due to the earlyclobber, reload must fix it up.  */
+      alternative_mask enabled = recog_data.enabled_alternatives;
       for (alt1 = 0; alt1 < recog_data.n_alternatives; alt1++)
 	{
+	  if (!TEST_BIT (enabled, alt1))
+	    continue;
 	  operand_alternative *op_alt1 = &recog_op_alt[alt1 * n_operands];
 	  if (op_alt1[use].matches == def
 	      || (use < n_operands - 1
@@ -688,30 +691,32 @@  make_early_clobber_and_input_conflicts (
 
   int n_alternatives = recog_data.n_alternatives;
   int n_operands = recog_data.n_operands;
+  alternative_mask enabled = recog_data.enabled_alternatives;
   operand_alternative *op_alt = recog_op_alt;
   for (alt = 0; alt < n_alternatives; alt++, op_alt += n_operands)
-    for (def = 0; def < n_operands; def++)
-      {
-	def_cl = NO_REGS;
-	if (op_alt[def].earlyclobber)
-	  {
-	    if (op_alt[def].anything_ok)
-	      def_cl = ALL_REGS;
-	    else
-	      def_cl = op_alt[def].cl;
-	    check_and_make_def_conflict (alt, def, def_cl);
-	  }
-	if ((def_match = op_alt[def].matches) >= 0
-	    && (op_alt[def_match].earlyclobber
-		|| op_alt[def].earlyclobber))
-	  {
-	    if (op_alt[def_match].anything_ok)
-	      def_cl = ALL_REGS;
-	    else
-	      def_cl = op_alt[def_match].cl;
-	    check_and_make_def_conflict (alt, def, def_cl);
-	  }
-      }
+    if (TEST_BIT (enabled, alt))
+      for (def = 0; def < n_operands; def++)
+	{
+	  def_cl = NO_REGS;
+	  if (op_alt[def].earlyclobber)
+	    {
+	      if (op_alt[def].anything_ok)
+		def_cl = ALL_REGS;
+	      else
+		def_cl = op_alt[def].cl;
+	      check_and_make_def_conflict (alt, def, def_cl);
+	    }
+	  if ((def_match = op_alt[def].matches) >= 0
+	      && (op_alt[def_match].earlyclobber
+		  || op_alt[def].earlyclobber))
+	    {
+	      if (op_alt[def_match].anything_ok)
+		def_cl = ALL_REGS;
+	      else
+		def_cl = op_alt[def_match].cl;
+	      check_and_make_def_conflict (alt, def, def_cl);
+	    }
+	}
 }
 
 /* Mark early clobber hard registers of the current INSN as live (if
Index: gcc/config/i386/i386.c
===================================================================
--- gcc/config/i386/i386.c	2014-05-31 08:54:12.236234739 +0100
+++ gcc/config/i386/i386.c	2014-05-31 09:07:21.682714985 +0100
@@ -5874,8 +5874,11 @@  ix86_legitimate_combined_insn (rtx insn)
 	  /* Operand has no constraints, anything is OK.  */
  	  win = !n_alternatives;
 
+	  alternative_mask enabled = recog_data.enabled_alternatives;
 	  for (j = 0; j < n_alternatives; j++, op_alt += n_operands)
 	    {
+	      if (!TEST_BIT (enabled, j))
+		continue;
 	      if (op_alt[i].anything_ok
 		  || (op_alt[i].matches != -1
 		      && operands_match_p