diff mbox

[6/7] Tidy IRA move costs

Message ID 87mx4pfr4m.fsf@talisman.home
State New
Headers show

Commit Message

Richard Sandiford May 30, 2012, 6:41 p.m. UTC 
This patch makes the original move_cost calculation match the value
currently calculated for ira_register_move_cost, asserting that the
"IRA code" now has nothing to do.

It seems like we really ought to be preserving the contains_reg_of_mode
part of the original move_cost check, i.e.:

		if (contains_reg_of_mode[*p2][mode])
		    && ira_class_hard_regs_num[*p2] > 0
		    && (ira_reg_class_max_nregs[*p2][mode]
			<= ira_class_hard_regs_num[*p2]))

etc.  But that changes the cc1 .ii output for x86_64, so the current
costs really do include the costs for subclasses that don't contain
registers of a particular mode.  I think adding the check "back"
should be a separate patch (by someone who can test the performance!).

A strict conversion for may_move_in_cost and may_move_out_cost would
be to convert the two instances of:

	      may_move_in_cost[mode][cl1][cl2] = 65535;
	      may_move_out_cost[mode][cl1][cl2] = 65535;

to:

	      if (ira_class_hard_regs_num[cl2] > 0
		  && ira_class_subset_p[cl1][cl2])
		may_move_in_cost[mode][cl1][cl2] = 0;
	      else
		may_move_in_cost[mode][cl1][cl2] = 65535;

	      if (ira_class_hard_regs_num[cl2] > 0
		  && ira_class_subset_p[cl2][cl1])
		may_move_in_cost[mode][cl1][cl2] = 0;
	      else
		may_move_out_cost[mode][cl1][cl2] = 65535;

because here too the current IRA costs don't take
contains_reg_of_mode into account.  But that change wouldn't
really make sense, because cl2 represents different things
for the "in" and "out" cost (the operand class and the allocation
class respectively).  The cc1 .ii output is the same either way,
so for this one I've just added the contains_reg_of_mode tests
to the IRA version.

It might seem odd to commit these asserts and then remove them
in the next patch.  But I'd like to commit them anyway so that,
if this series does mess things up, the asserts can help show why.

Richard


gcc/
	* ira.c (init_move_cost): Adjust choice of subclasses to match
	the current ira_init_register_move_cost choice.  Use
	ira_class_subset_p instead of reg_class_subset_p.
	(ira_init_register_move_cost): Assert that move_cost,
	may_move_in_cost and may_move_out_cost already hold the desired
	values for their ira_* equivalents.  For the latter two,
	ignore classes that can't store a register of the given mode.

Comments

Vladimir Makarov May 31, 2012, 2:20 a.m. UTC | #1 
On 05/30/2012 02:41 PM, Richard Sandiford wrote:
> This patch makes the original move_cost calculation match the value
> currently calculated for ira_register_move_cost, asserting that the
> "IRA code" now has nothing to do.
>
> It seems like we really ought to be preserving the contains_reg_of_mode
> part of the original move_cost check, i.e.:
>
> 		if (contains_reg_of_mode[*p2][mode])
> 		&&  ira_class_hard_regs_num[*p2]>  0
> 		&&  (ira_reg_class_max_nregs[*p2][mode]
> 			<= ira_class_hard_regs_num[*p2]))
>
> etc.  But that changes the cc1 .ii output for x86_64, so the current
> costs really do include the costs for subclasses that don't contain
> registers of a particular mode.  I think adding the check "back"
> should be a separate patch (by someone who can test the performance!).
>
> A strict conversion for may_move_in_cost and may_move_out_cost would
> be to convert the two instances of:
>
> 	      may_move_in_cost[mode][cl1][cl2] = 65535;
> 	      may_move_out_cost[mode][cl1][cl2] = 65535;
>
> to:
>
> 	      if (ira_class_hard_regs_num[cl2]>  0
> 		&&  ira_class_subset_p[cl1][cl2])
> 		may_move_in_cost[mode][cl1][cl2] = 0;
> 	      else
> 		may_move_in_cost[mode][cl1][cl2] = 65535;
>
> 	      if (ira_class_hard_regs_num[cl2]>  0
> 		&&  ira_class_subset_p[cl2][cl1])
> 		may_move_in_cost[mode][cl1][cl2] = 0;
> 	      else
> 		may_move_out_cost[mode][cl1][cl2] = 65535;
>
> because here too the current IRA costs don't take
> contains_reg_of_mode into account.  But that change wouldn't
> really make sense, because cl2 represents different things
> for the "in" and "out" cost (the operand class and the allocation
> class respectively).  The cc1 .ii output is the same either way,
> so for this one I've just added the contains_reg_of_mode tests
> to the IRA version.
>
> It might seem odd to commit these asserts and then remove them
> in the next patch.  But I'd like to commit them anyway so that,
> if this series does mess things up, the asserts can help show why.
>
> Richard
>
>
> gcc/
> 	* ira.c (init_move_cost): Adjust choice of subclasses to match
> 	the current ira_init_register_move_cost choice.  Use
> 	ira_class_subset_p instead of reg_class_subset_p.
> 	(ira_init_register_move_cost): Assert that move_cost,
> 	may_move_in_cost and may_move_out_cost already hold the desired
> 	values for their ira_* equivalents.  For the latter two,
> 	ignore classes that can't store a register of the given mode.
>
>
Ok. Thanks.
diff mbox

Patch

Index: gcc/ira.c
===================================================================
--- gcc/ira.c	2012-05-29 19:35:14.000000000 +0100
+++ gcc/ira.c	2012-05-30 18:56:57.930913292 +0100
@@ -1510,25 +1510,27 @@  init_move_cost (enum machine_mode mode)
 
 	      for (p2 = &reg_class_subclasses[cl2][0];
 		   *p2 != LIM_REG_CLASSES; p2++)
-		if (*p2 != cl1 && contains_reg_of_mode[*p2][mode])
+		if (ira_class_hard_regs_num[*p2] > 0
+		    && (ira_reg_class_max_nregs[*p2][mode]
+			<= ira_class_hard_regs_num[*p2]))
 		  cost = MAX (cost, move_cost[mode][cl1][*p2]);
 
 	      for (p1 = &reg_class_subclasses[cl1][0];
 		   *p1 != LIM_REG_CLASSES; p1++)
-		if (*p1 != cl2 && contains_reg_of_mode[*p1][mode])
+		if (ira_class_hard_regs_num[*p1] > 0
+		    && (ira_reg_class_max_nregs[*p1][mode]
+			<= ira_class_hard_regs_num[*p1]))
 		  cost = MAX (cost, move_cost[mode][*p1][cl2]);
 
 	      ira_assert (cost <= 65535);
 	      move_cost[mode][cl1][cl2] = cost;
 
-	      if (reg_class_subset_p ((enum reg_class) cl1,
-				      (enum reg_class) cl2))
+	      if (ira_class_subset_p[cl1][cl2])
 		may_move_in_cost[mode][cl1][cl2] = 0;
 	      else
 		may_move_in_cost[mode][cl1][cl2] = cost;
 
-	      if (reg_class_subset_p ((enum reg_class) cl2,
-				      (enum reg_class) cl1))
+	      if (ira_class_subset_p[cl2][cl1])
 		may_move_out_cost[mode][cl1][cl2] = 0;
 	      else
 		may_move_out_cost[mode][cl1][cl2] = cost;
@@ -1577,14 +1579,10 @@  ira_init_register_move_cost (enum machin
 				   reg_class_contents[cl2]))
 	  for (cl3 = 0; cl3 < N_REG_CLASSES; cl3++)
 	    {
-	      if (ira_max_register_move_cost[mode][cl2][cl3]
-		  < ira_register_move_cost[mode][cl1][cl3])
-		ira_max_register_move_cost[mode][cl2][cl3]
-		  = ira_register_move_cost[mode][cl1][cl3];
-	      if (ira_max_register_move_cost[mode][cl3][cl2]
-		  < ira_register_move_cost[mode][cl3][cl1])
-		ira_max_register_move_cost[mode][cl3][cl2]
-		  = ira_register_move_cost[mode][cl3][cl1];
+	      gcc_assert (ira_register_move_cost[mode][cl2][cl3]
+			  >= ira_register_move_cost[mode][cl1][cl3]);
+	      gcc_assert (ira_register_move_cost[mode][cl3][cl2]
+			  >= ira_register_move_cost[mode][cl3][cl1]);
 	    }
   ira_may_move_in_cost[mode]
     = (move_table *) xmalloc (sizeof (move_table) * N_REG_CLASSES);
@@ -1603,27 +1601,25 @@  ira_init_register_move_cost (enum machin
   memcpy (ira_max_may_move_out_cost[mode], ira_max_register_move_cost[mode],
 	  sizeof (move_table) * N_REG_CLASSES);
   for (cl1 = 0; cl1 < N_REG_CLASSES; cl1++)
-    {
+    if (contains_reg_of_mode[cl1][mode])
       for (cl2 = 0; cl2 < N_REG_CLASSES; cl2++)
 	{
-	  if (ira_class_hard_regs_num[cl2] == 0)
+	  if (!contains_reg_of_mode[cl2][mode]
+	      || ira_class_hard_regs_num[cl2] == 0)
 	    continue;
 	  if (ira_class_subset_p[cl1][cl2])
-	    ira_may_move_in_cost[mode][cl1][cl2] = 0;
+	    gcc_assert (ira_may_move_in_cost[mode][cl1][cl2] == 0);
 	  if (ira_class_subset_p[cl2][cl1])
-	    ira_may_move_out_cost[mode][cl1][cl2] = 0;
+	    gcc_assert (ira_may_move_out_cost[mode][cl1][cl2] == 0);
 	  if (ira_class_subset_p[cl1][cl2])
 	    ira_max_may_move_in_cost[mode][cl1][cl2] = 0;
 	  if (ira_class_subset_p[cl2][cl1])
 	    ira_max_may_move_out_cost[mode][cl1][cl2] = 0;
-	  ira_register_move_cost[mode][cl1][cl2]
-	    = ira_max_register_move_cost[mode][cl1][cl2];
-	  ira_may_move_in_cost[mode][cl1][cl2]
-	    = ira_max_may_move_in_cost[mode][cl1][cl2];
-	  ira_may_move_out_cost[mode][cl1][cl2]
-	    = ira_max_may_move_out_cost[mode][cl1][cl2];
+	  gcc_assert (ira_may_move_in_cost[mode][cl1][cl2]
+		      == ira_max_may_move_in_cost[mode][cl1][cl2]);
+	  gcc_assert (ira_may_move_out_cost[mode][cl1][cl2]
+		      == ira_max_may_move_out_cost[mode][cl1][cl2]);
 	}
-    }
 }