diff mbox series

LRA: Don't use 0 as initialization for sp_offset

Message ID d0103f49-7bdf-d419-c4ae-84347b961228@suse.de
State New
Headers show
Series LRA: Don't use 0 as initialization for sp_offset | expand

Commit Message

Michael Matz Aug. 22, 2024, 3:44 p.m. UTC 
this is part of making m68k work with LRA.  See PR116374.
m68k has the property that sometimes the elimation offset
between %sp and %argptr is zero.  During setting up elimination
infrastructure it's changes between sp_offset and previous_offset
that feed into insns_with_changed_offsets that ultimately will
setup looking at the instructions so marked.

But the initial values for sp_offset and previous_offset are
also zero.  So if the targets INITIAL_ELIMINATION_OFFSET (called
in update_reg_eliminate) is zero then nothing changes, the
instructions in question don't get into the list to consider and
the sp_offset tracking goes wrong.

Solve this by initializing those member with -1 instead of zero.
An initial offset of that value seems very unlikely, as it's
in word-sized increments.  This then also reveals a problem in
eliminate_regs_in_insn where it always uses sp_offset-previous_offset
as offset adjustment, even in the first_p pass.  That was harmless
when previous_offset was uninitialized as zero.  But all the other
code uses a different idiom of checking for first_p (or rather
update_p which is !replace_p&&!first_p), and using sp_offset directly.
So use that as well in eliminate_regs_in_insn.

	PR target/116374
	* lra-eliminations.cc (init_elim_table): Use -1 as initializer.
	(update_reg_eliminate): Accept -1 as not-yet-used marker.
	(eliminate_regs_in_insn): Use previous_sp_offset only when
	not first_p.
---
 gcc/lra-eliminations.cc | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)
---

Regstrapped on x86-64-linux.  Okay?

Comments

Jeff Law Aug. 25, 2024, 2:05 p.m. UTC | #1 
On 8/22/24 9:44 AM, Michael Matz wrote:
> this is part of making m68k work with LRA.  See PR116374.
> m68k has the property that sometimes the elimation offset
> between %sp and %argptr is zero.  During setting up elimination
> infrastructure it's changes between sp_offset and previous_offset
> that feed into insns_with_changed_offsets that ultimately will
> setup looking at the instructions so marked.
> 
> But the initial values for sp_offset and previous_offset are
> also zero.  So if the targets INITIAL_ELIMINATION_OFFSET (called
> in update_reg_eliminate) is zero then nothing changes, the
> instructions in question don't get into the list to consider and
> the sp_offset tracking goes wrong.
> 
> Solve this by initializing those member with -1 instead of zero.
> An initial offset of that value seems very unlikely, as it's
> in word-sized increments.  This then also reveals a problem in
> eliminate_regs_in_insn where it always uses sp_offset-previous_offset
> as offset adjustment, even in the first_p pass.  That was harmless
> when previous_offset was uninitialized as zero.  But all the other
> code uses a different idiom of checking for first_p (or rather
> update_p which is !replace_p&&!first_p), and using sp_offset directly.
> So use that as well in eliminate_regs_in_insn.
> 
> 	PR target/116374
> 	* lra-eliminations.cc (init_elim_table): Use -1 as initializer.
> 	(update_reg_eliminate): Accept -1 as not-yet-used marker.
> 	(eliminate_regs_in_insn): Use previous_sp_offset only when
> 	not first_p.
OK
jeff
diff mbox series

Patch

diff --git a/gcc/lra-eliminations.cc b/gcc/lra-eliminations.cc
index 5bed259cffe..96772f2904a 100644
--- a/gcc/lra-eliminations.cc
+++ b/gcc/lra-eliminations.cc
@@ -969,7 +969,8 @@  eliminate_regs_in_insn (rtx_insn *insn, bool replace_p, bool first_p,
 	  if (! replace_p)
 	    {
 	      if (known_eq (update_sp_offset, 0))
-		offset += (ep->offset - ep->previous_offset);
+		offset += (!first_p
+			   ? ep->offset - ep->previous_offset : ep->offset);
 	      if (ep->to_rtx == stack_pointer_rtx)
 		{
 		  if (first_p)
@@ -1212,7 +1213,7 @@  update_reg_eliminate (bitmap insns_with_changed_offsets)
 	      if (lra_dump_file != NULL)
 		fprintf (lra_dump_file, "    Using elimination %d to %d now\n",
 			 ep1->from, ep1->to);
-	      lra_assert (known_eq (ep1->previous_offset, 0));
+	      lra_assert (known_eq (ep1->previous_offset, -1));
 	      ep1->previous_offset = ep->offset;
 	    }
 	  else
@@ -1283,7 +1284,7 @@  init_elim_table (void)
   for (ep = reg_eliminate, ep1 = reg_eliminate_1;
        ep < &reg_eliminate[NUM_ELIMINABLE_REGS]; ep++, ep1++)
     {
-      ep->offset = ep->previous_offset = 0;
+      ep->offset = ep->previous_offset = -1;
       ep->from = ep1->from;
       ep->to = ep1->to;
       value_p = (targetm.can_eliminate (ep->from, ep->to)