lra: Stop registers being incorrectly marked live v2 [PR92989]

Message ID mptftf6ka1m.fsf@arm.com
State New
Headers show
Series
  • lra: Stop registers being incorrectly marked live v2 [PR92989]
Related show

Commit Message

Richard Sandiford Feb. 19, 2020, 12:59 p.m.
This PR is about a case in which the clobbers at the start of
an EH receiver can lead to registers becoming unnecessarily
live in predecessor blocks.  My first attempt at fixing this
made sure that we update the bb liveness info based on the
real live set:

  http://gcc.gnu.org/g:e648e57efca6ce6d751ef8c2038608817b514fb4

But it turns out that the clobbered registers were also added to
the "gen" set of LRA's private liveness problem, where "gen" in
this context means "generates a requirement for a live value".
So the clobbered registers could still end up live via that
mechanism instead.

This patch therefore reverts the patch above and takes the other
approach floated in the original patch description: model the full
clobber by making the registers live and then dead again.

There's no specific need to revert the original patch, since the
code should no longer be sensitive to the order of the bb liveness
update and the modelling of the clobber.  But given that there's
no specific need to keep the original patch either, it seemed better
to restore the code to the more well-tested order.

Tested on aarch64-linux-gnu and x86_64-linux-gnu.  OK to install?

Richard


2020-02-19  Richard Sandiford  <richard.sandiford@arm.com>

gcc/
	PR rtl-optimization/PR92989
	* lra-lives.c (process_bb_lives): Restore the original order
	of the bb liveness update.  Call make_hard_regno_dead for each
	register clobbered at the start of an EH receiver.
---
 gcc/lra-lives.c | 109 +++++++++++++++++++++++++-----------------------
 1 file changed, 57 insertions(+), 52 deletions(-)

Comments

Jeff Law Feb. 21, 2020, 8:33 p.m. | #1
On Wed, 2020-02-19 at 12:59 +0000, Richard Sandiford wrote:
> This PR is about a case in which the clobbers at the start of

> an EH receiver can lead to registers becoming unnecessarily

> live in predecessor blocks.  My first attempt at fixing this

> made sure that we update the bb liveness info based on the

> real live set:

> 

>   http://gcc.gnu.org/g:e648e57efca6ce6d751ef8c2038608817b514fb4

> 

> But it turns out that the clobbered registers were also added to

> the "gen" set of LRA's private liveness problem, where "gen" in

> this context means "generates a requirement for a live value".

> So the clobbered registers could still end up live via that

> mechanism instead.

> 

> This patch therefore reverts the patch above and takes the other

> approach floated in the original patch description: model the full

> clobber by making the registers live and then dead again.

> 

> There's no specific need to revert the original patch, since the

> code should no longer be sensitive to the order of the bb liveness

> update and the modelling of the clobber.  But given that there's

> no specific need to keep the original patch either, it seemed better

> to restore the code to the more well-tested order.

> 

> Tested on aarch64-linux-gnu and x86_64-linux-gnu.  OK to install?

> 

> Richard

> 

> 

> 2020-02-19  Richard Sandiford  <richard.sandiford@arm.com>

> 

> gcc/

> 	PR rtl-optimization/PR92989

> 	* lra-lives.c (process_bb_lives): Restore the original order

> 	of the bb liveness update.  Call make_hard_regno_dead for each

> 	register clobbered at the start of an EH receiver.

THanks.  Installed.
jeff
>

Patch

diff --git a/gcc/lra-lives.c b/gcc/lra-lives.c
index f439e899395..c8780779a11 100644
--- a/gcc/lra-lives.c
+++ b/gcc/lra-lives.c
@@ -1023,57 +1023,6 @@  process_bb_lives (basic_block bb, int &curr_point, bool dead_insn_p)
 	make_hard_regno_live (regno);
       }
 
-  bool live_change_p = false;
-  /* Check if bb border live info was changed.  */
-  unsigned int live_pseudos_num = 0;
-  EXECUTE_IF_SET_IN_BITMAP (df_get_live_in (bb),
-			    FIRST_PSEUDO_REGISTER, j, bi)
-    {
-      live_pseudos_num++;
-      if (! sparseset_bit_p (pseudos_live, j))
-	{
-	  live_change_p = true;
-	  if (lra_dump_file != NULL)
-	    fprintf (lra_dump_file,
-		     "  r%d is removed as live at bb%d start\n", j, bb->index);
-	  break;
-	}
-    }
-  if (! live_change_p
-      && sparseset_cardinality (pseudos_live) != live_pseudos_num)
-    {
-      live_change_p = true;
-      if (lra_dump_file != NULL)
-	EXECUTE_IF_SET_IN_SPARSESET (pseudos_live, j)
-	  if (! bitmap_bit_p (df_get_live_in (bb), j))
-	    fprintf (lra_dump_file,
-		     "  r%d is added to live at bb%d start\n", j, bb->index);
-    }
-
-  /* The order of this code and the code below is important.  At this
-     point hard_regs_live does genuinely contain only live registers.
-     Below we pretend other hard registers are live in order to create
-     conflicts with pseudos, but this fake live set shouldn't leak out
-     into the df info.  */
-  for (i = 0; HARD_REGISTER_NUM_P (i); ++i)
-    {
-      if (!TEST_HARD_REG_BIT (hard_regs_live, i))
-	continue;
-
-      if (!TEST_HARD_REG_BIT (hard_regs_spilled_into, i))
-	continue;
-
-      if (bitmap_bit_p (df_get_live_in (bb), i))
-	continue;
-
-      live_change_p = true;
-      if (lra_dump_file)
-	fprintf (lra_dump_file,
-		 "  hard reg r%d is added to live at bb%d start\n", i,
-		 bb->index);
-      bitmap_set_bit (df_get_live_in (bb), i);
-    }
-
   /* Pseudos can't go in stack regs at the start of a basic block that
      is reached by an abnormal edge.  Likewise for registers that are at
      least partly call clobbered, because caller-save, fixup_abnormal_edges
@@ -1081,11 +1030,14 @@  process_bb_lives (basic_block bb, int &curr_point, bool dead_insn_p)
      handle such pseudos live across such edges.  */
   if (bb_has_abnormal_pred (bb))
     {
+      HARD_REG_SET clobbers;
+
+      CLEAR_HARD_REG_SET (clobbers);
 #ifdef STACK_REGS
       EXECUTE_IF_SET_IN_SPARSESET (pseudos_live, px)
 	lra_reg_info[px].no_stack_p = true;
       for (px = FIRST_STACK_REG; px <= LAST_STACK_REG; px++)
-	make_hard_regno_live (px);
+	SET_HARD_REG_BIT (clobbers, px);
 #endif
       /* No need to record conflicts for call clobbered regs if we
 	 have nonlocal labels around, as we don't ever try to
@@ -1105,9 +1057,43 @@  process_bb_lives (basic_block bb, int &curr_point, bool dead_insn_p)
 		  && !HARD_REGISTER_P (pic_offset_table_rtx))
 #endif
 	      )
+	    SET_HARD_REG_BIT (clobbers, px);
+
+      clobbers &= ~hard_regs_live;
+      for (px = 0; HARD_REGISTER_NUM_P (px); px++)
+	if (TEST_HARD_REG_BIT (clobbers, px))
+	  {
 	    make_hard_regno_live (px);
+	    make_hard_regno_dead (px);
+	  }
     }
 
+  bool live_change_p = false;
+  /* Check if bb border live info was changed.  */
+  unsigned int live_pseudos_num = 0;
+  EXECUTE_IF_SET_IN_BITMAP (df_get_live_in (bb),
+			    FIRST_PSEUDO_REGISTER, j, bi)
+    {
+      live_pseudos_num++;
+      if (! sparseset_bit_p (pseudos_live, j))
+	{
+	  live_change_p = true;
+	  if (lra_dump_file != NULL)
+	    fprintf (lra_dump_file,
+		     "  r%d is removed as live at bb%d start\n", j, bb->index);
+	  break;
+	}
+    }
+  if (! live_change_p
+      && sparseset_cardinality (pseudos_live) != live_pseudos_num)
+    {
+      live_change_p = true;
+      if (lra_dump_file != NULL)
+	EXECUTE_IF_SET_IN_SPARSESET (pseudos_live, j)
+	  if (! bitmap_bit_p (df_get_live_in (bb), j))
+	    fprintf (lra_dump_file,
+		     "  r%d is added to live at bb%d start\n", j, bb->index);
+    }
   /* See if we'll need an increment at the end of this basic block.
      An increment is needed if the PSEUDOS_LIVE set is not empty,
      to make sure the finish points are set up correctly.  */
@@ -1127,6 +1113,25 @@  process_bb_lives (basic_block bb, int &curr_point, bool dead_insn_p)
 	check_pseudos_live_through_calls (j, last_call_abi);
     }
 
+  for (i = 0; HARD_REGISTER_NUM_P (i); ++i)
+    {
+      if (!TEST_HARD_REG_BIT (hard_regs_live, i))
+	continue;
+
+      if (!TEST_HARD_REG_BIT (hard_regs_spilled_into, i))
+	continue;
+
+      if (bitmap_bit_p (df_get_live_in (bb), i))
+	continue;
+
+      live_change_p = true;
+      if (lra_dump_file)
+	fprintf (lra_dump_file,
+		 "  hard reg r%d is added to live at bb%d start\n", i,
+		 bb->index);
+      bitmap_set_bit (df_get_live_in (bb), i);
+    }
+
   if (need_curr_point_incr)
     next_program_point (curr_point, freq);