diff mbox series

[2/5] Simplify ira_setup_alts

Message ID mptfto3m4yd.fsf@arm.com
State New
Headers show
Series Tweak IRA handling of tying and earlyclobbers | expand

Commit Message

Richard Sandiford June 21, 2019, 1:40 p.m. UTC
ira_setup_alts has its own code to calculate the start of the
constraint string for each operand/alternative combination,
but preprocess_constraints now provides that information in (almost)
constant time for non-asm instructions.  Using it here should speed
up the common case at the cost of potentially slowing down the handling
of asm statements.

The real reason for doing this is that a later patch wants to use
more of the operand_alternative information.


2019-06-21  Richard Sandiford  <richard.sandiford@arm.com>

gcc/
	* ira.c (ira_setup_alts): Use preprocess_constraints to get the
	constraint string for each operand/alternative combo.  Only handle
	'%' at the start of constraint strings, and look for it outside
	the main loop.

Comments

Vladimir Makarov June 24, 2019, 2:30 p.m. UTC | #1
On 2019-06-21 9:40 a.m., Richard Sandiford wrote:
> ira_setup_alts has its own code to calculate the start of the
> constraint string for each operand/alternative combination,
> but preprocess_constraints now provides that information in (almost)
> constant time for non-asm instructions.  Using it here should speed
> up the common case at the cost of potentially slowing down the handling
> of asm statements.

The documentation says that '%' should be the very first constraint 
character.  But I think there is a possibility that somebody can forget 
this and put a blank before '%' and effect of this would be very hard to 
find as still the correct code would be generated although the code 
might be slower.  That was my thoughts why I processed all constraint 
string.

It is hard to for me to say what the probability of this can be. I guess 
it is tiny.  So the patch is ok for me.

> The real reason for doing this is that a later patch wants to use
> more of the operand_alternative information.
>
> 2019-06-21  Richard Sandiford  <richard.sandiford@arm.com>
>
> gcc/
> 	* ira.c (ira_setup_alts): Use preprocess_constraints to get the
> 	constraint string for each operand/alternative combo.  Only handle
> 	'%' at the start of constraint strings, and look for it outside
> 	the main loop.
>
> Index: gcc/ira.c
> ===================================================================
> --- gcc/ira.c	2019-06-21 14:34:05.887715020 +0100
> +++ gcc/ira.c	2019-06-21 14:34:09.455685354 +0100
> @@ -1791,60 +1791,42 @@ setup_prohibited_mode_move_regs (void)
>   alternative_mask
>   ira_setup_alts (rtx_insn *insn)
>   {
> -  /* MAP nalt * nop -> start of constraints for given operand and
> -     alternative.  */
> -  static vec<const char *> insn_constraints;
>     int nop, nalt;
>     bool curr_swapped;
>     const char *p;
>     int commutative = -1;
>   
>     extract_insn (insn);
> +  preprocess_constraints (insn);
>     alternative_mask preferred = get_preferred_alternatives (insn);
>     alternative_mask alts = 0;
> -  insn_constraints.release ();
> -  insn_constraints.safe_grow_cleared (recog_data.n_operands
> -				      * recog_data.n_alternatives + 1);
>     /* Check that the hard reg set is enough for holding all
>        alternatives.  It is hard to imagine the situation when the
>        assertion is wrong.  */
>     ira_assert (recog_data.n_alternatives
>   	      <= (int) MAX (sizeof (HARD_REG_ELT_TYPE) * CHAR_BIT,
>   			    FIRST_PSEUDO_REGISTER));
> +  for (nop = 0; nop < recog_data.n_operands; nop++)
> +    if (recog_data.constraints[nop][0] == '%')
> +      {
> +	commutative = nop;
> +	break;
> +      }
>     for (curr_swapped = false;; curr_swapped = true)
>       {
> -      /* Calculate some data common for all alternatives to speed up the
> -	 function.  */
> -      for (nop = 0; nop < recog_data.n_operands; nop++)
> -	{
> -	  for (nalt = 0, p = recog_data.constraints[nop];
> -	       nalt < recog_data.n_alternatives;
> -	       nalt++)
> -	    {
> -	      insn_constraints[nop * recog_data.n_alternatives + nalt] = p;
> -	      while (*p && *p != ',')
> -		{
> -		  /* We only support one commutative marker, the first
> -		     one.  We already set commutative above.  */
> -		  if (*p == '%' && commutative < 0)
> -		    commutative = nop;
> -		  p++;
> -		}
> -	      if (*p)
> -		p++;
> -	    }
> -	}
>         for (nalt = 0; nalt < recog_data.n_alternatives; nalt++)
>   	{
>   	  if (!TEST_BIT (preferred, nalt) || TEST_BIT (alts, nalt))
>   	    continue;
>   
> +	  const operand_alternative *op_alt
> +	    = &recog_op_alt[nalt * recog_data.n_operands];
>   	  for (nop = 0; nop < recog_data.n_operands; nop++)
>   	    {
>   	      int c, len;
>   
>   	      rtx op = recog_data.operand[nop];
> -	      p = insn_constraints[nop * recog_data.n_alternatives + nalt];
> +	      p = op_alt[nop].constraint;
>   	      if (*p == 0 || *p == ',')
>   		continue;
>
diff mbox series

Patch

Index: gcc/ira.c
===================================================================
--- gcc/ira.c	2019-06-21 14:34:05.887715020 +0100
+++ gcc/ira.c	2019-06-21 14:34:09.455685354 +0100
@@ -1791,60 +1791,42 @@  setup_prohibited_mode_move_regs (void)
 alternative_mask
 ira_setup_alts (rtx_insn *insn)
 {
-  /* MAP nalt * nop -> start of constraints for given operand and
-     alternative.  */
-  static vec<const char *> insn_constraints;
   int nop, nalt;
   bool curr_swapped;
   const char *p;
   int commutative = -1;
 
   extract_insn (insn);
+  preprocess_constraints (insn);
   alternative_mask preferred = get_preferred_alternatives (insn);
   alternative_mask alts = 0;
-  insn_constraints.release ();
-  insn_constraints.safe_grow_cleared (recog_data.n_operands
-				      * recog_data.n_alternatives + 1);
   /* Check that the hard reg set is enough for holding all
      alternatives.  It is hard to imagine the situation when the
      assertion is wrong.  */
   ira_assert (recog_data.n_alternatives
 	      <= (int) MAX (sizeof (HARD_REG_ELT_TYPE) * CHAR_BIT,
 			    FIRST_PSEUDO_REGISTER));
+  for (nop = 0; nop < recog_data.n_operands; nop++)
+    if (recog_data.constraints[nop][0] == '%')
+      {
+	commutative = nop;
+	break;
+      }
   for (curr_swapped = false;; curr_swapped = true)
     {
-      /* Calculate some data common for all alternatives to speed up the
-	 function.  */
-      for (nop = 0; nop < recog_data.n_operands; nop++)
-	{
-	  for (nalt = 0, p = recog_data.constraints[nop];
-	       nalt < recog_data.n_alternatives;
-	       nalt++)
-	    {
-	      insn_constraints[nop * recog_data.n_alternatives + nalt] = p;
-	      while (*p && *p != ',')
-		{
-		  /* We only support one commutative marker, the first
-		     one.  We already set commutative above.  */
-		  if (*p == '%' && commutative < 0)
-		    commutative = nop;
-		  p++;
-		}
-	      if (*p)
-		p++;
-	    }
-	}
       for (nalt = 0; nalt < recog_data.n_alternatives; nalt++)
 	{
 	  if (!TEST_BIT (preferred, nalt) || TEST_BIT (alts, nalt))
 	    continue;
 
+	  const operand_alternative *op_alt
+	    = &recog_op_alt[nalt * recog_data.n_operands];
 	  for (nop = 0; nop < recog_data.n_operands; nop++)
 	    {
 	      int c, len;
 
 	      rtx op = recog_data.operand[nop];
-	      p = insn_constraints[nop * recog_data.n_alternatives + nalt];
+	      p = op_alt[nop].constraint;
 	      if (*p == 0 || *p == ',')
 		continue;