diff mbox series

[2/6] ira: Add comments and fix move_spill_restore calculation

Message ID mptfsq02a7c.fsf@arm.com
State Committed
Commit 909a4b4764c4f270f09ccb2a950c91b21ed7b33a
Headers
Series ira: Fix performance regression in exchange2 [PR98782] |

Commit Message

Richard Sandiford Jan. 6, 2022, 2:46 p.m. UTC 
  This patch adds comments to describe each use of ira_loop_border_costs.
I think this highlights that move_spill_restore was using the wrong cost
in one case, which came from tranposing [0] and [1] in the original
(pre-ira_loop_border_costs) ira_memory_move_cost expressions.  The
difference would only be noticeable on targets that distinguish between
load and store costs.

gcc/
	PR rtl-optimization/98782
	* ira-color.c (color_pass): Add comments to describe the spill costs.
	(move_spill_restore): Likewise.  Fix reversed calculation.
---
 gcc/ira-color.c | 28 +++++++++++++++++++++++++++-
 1 file changed, 27 insertions(+), 1 deletion(-)

Comments

Vladimir Makarov Jan. 7, 2022, 2:39 p.m. UTC | #1 
On 2022-01-06 09:46, Richard Sandiford wrote:
> This patch adds comments to describe each use of ira_loop_border_costs.
> I think this highlights that move_spill_restore was using the wrong cost
> in one case, which came from tranposing [0] and [1] in the original
> (pre-ira_loop_border_costs) ira_memory_move_cost expressions.  The
> difference would only be noticeable on targets that distinguish between
> load and store costs.
>
> gcc/
> 	PR rtl-optimization/98782
> 	* ira-color.c (color_pass): Add comments to describe the spill costs.
> 	(move_spill_restore): Likewise.  Fix reversed calculation.
OK for me.  Thank you for fixing the cost typo.
diff mbox series

Patch

diff --git a/gcc/ira-color.c b/gcc/ira-color.c
index 66c11710b97..e7433312675 100644
--- a/gcc/ira-color.c
+++ b/gcc/ira-color.c
@@ -3479,6 +3479,13 @@  color_pass (ira_loop_tree_node_t loop_tree_node)
 	    }
 	  else if (hard_regno < 0)
 	    {
+	      /* If we allocate a register to SUBLOOP_ALLOCNO, we'll need
+		 to load the register on entry to the subloop and store
+		 the register back on exit from the subloop.  This incurs
+		 a fixed cost for all registers.  Since UPDATED_MEMORY_COST
+		 is (and should only be) used relative to the register costs
+		 for the same allocno, we can subtract this shared register
+		 cost from the memory cost.  */
 	      ira_loop_border_costs border_costs (subloop_allocno);
 	      ALLOCNO_UPDATED_MEMORY_COST (subloop_allocno)
 		-= border_costs.spill_outside_loop_cost ();
@@ -3503,6 +3510,9 @@  color_pass (ira_loop_tree_node_t loop_tree_node)
 		  > ALLOCNO_UPDATED_HARD_REG_COSTS (subloop_allocno)[index])
 		ALLOCNO_UPDATED_CLASS_COST (subloop_allocno)
 		  = ALLOCNO_UPDATED_HARD_REG_COSTS (subloop_allocno)[index];
+	      /* If we spill SUBLOOP_ALLOCNO, we'll need to store HARD_REGNO
+		 on entry to the subloop and restore HARD_REGNO on exit from
+		 the subloop.  */
 	      ALLOCNO_UPDATED_MEMORY_COST (subloop_allocno)
 		+= border_costs.spill_inside_loop_cost ();
 	    }
@@ -3601,9 +3611,17 @@  move_spill_restore (void)
 			  : ALLOCNO_HARD_REG_COSTS (subloop_allocno)[index]));
 	      ira_loop_border_costs border_costs (subloop_allocno);
 	      if ((hard_regno2 = ALLOCNO_HARD_REGNO (subloop_allocno)) < 0)
-		cost -= border_costs.spill_outside_loop_cost ();
+		/* The register was spilled in the subloop.  If we spill
+		   it in the outer loop too then we'll no longer need to
+		   save the register on entry to the subloop and restore
+		   the register on exit from the subloop.  */
+		cost -= border_costs.spill_inside_loop_cost ();
 	      else
 		{
+		  /* The register was also allocated in the subloop.  If we
+		     spill it in the outer loop then we'll need to load the
+		     register on entry to the subloop and store the register
+		     back on exit from the subloop.  */
 		  cost += border_costs.spill_outside_loop_cost ();
 		  if (hard_regno2 != hard_regno)
 		    cost -= border_costs.move_between_loops_cost ();
@@ -3615,9 +3633,17 @@  move_spill_restore (void)
 	      ira_assert (rclass == ALLOCNO_CLASS (parent_allocno));
 	      ira_loop_border_costs border_costs (a);
 	      if ((hard_regno2 = ALLOCNO_HARD_REGNO (parent_allocno)) < 0)
+		/* The register was spilled in the parent loop.  If we spill
+		   it in this loop too then we'll no longer need to load the
+		   register on entry to this loop and save the register back
+		   on exit from this loop.  */
 		cost -= border_costs.spill_outside_loop_cost ();
 	      else
 		{
+		  /* The register was also allocated in the parent loop.
+		     If we spill it in this loop then we'll need to save
+		     the register on entry to this loop and restore the
+		     register on exit from this loop.  */
 		  cost += border_costs.spill_inside_loop_cost ();
 		  if (hard_regno2 != hard_regno)
 		    cost -= border_costs.move_between_loops_cost ();