diff mbox series

lra: Don't apply eliminations to allocated registers [PR116321]

Message ID mptzfp5rloj.fsf@arm.com
State New
Headers show
Series lra: Don't apply eliminations to allocated registers [PR116321] | expand

Commit Message

Richard Sandiford Aug. 22, 2024, 8:44 a.m. UTC 
The sequence of events in this PR is that:

- the function has many addresses in which only a single hard base
  register is acceptable.  Let's call the hard register H.

- IRA allocates that register to one of the pseudo base registers.
  Let's call the pseudo register P.

- Some of the other addresses that require H occur when P is still live.

- LRA therefore has to spill P.

- When it reallocates P, LRA chooses to use FRAME_POINTER_REGNUM,
  which has been eliminated to the stack pointer.  (This is ok,
  since the frame register is free.)

- Spilling P causes LRA to reprocess the instruction that uses P.

- When reprocessing the address that has P as its base, LRA first
  applies the new allocation, to get FRAME_POINTER_REGNUM,
  and then applies the elimination, to get the stack pointer.

The last step seems wrong: the elimination should only apply to
pre-existing uses of FRAME_POINTER_REGNUM, not to uses that result
from allocating pseudos.  Applying both means that we get the wrong
register number, and therefore the wrong class.

The PR is about an existing testcase that fails with LRA on m86k.

Tested on aarch64-linux-gnu, powerpc64le-linux-gnu, and
x86_64-linux-gnu.  Also tested by building at least one target
per CPU directory and checking that there were no asm changes or
new ICEs when compiling gcc.c-torture, gcc.dg, and g++.dg at -Os.
OK to install?

Richard


gcc/
	PR middle-end/116321
	* lra-constraints.cc (get_hard_regno): Only apply eliminations
	to existing hard registers.
	(get_reg_class): Likewise.
---
 gcc/lra-constraints.cc | 18 +++++++++---------
 1 file changed, 9 insertions(+), 9 deletions(-)

Comments

Vladimir Makarov Aug. 22, 2024, 3:12 p.m. UTC | #1 
On 8/22/24 04:44, Richard Sandiford wrote:
> The sequence of events in this PR is that:
>
> - the function has many addresses in which only a single hard base
>    register is acceptable.  Let's call the hard register H.
>
> - IRA allocates that register to one of the pseudo base registers.
>    Let's call the pseudo register P.
>
> - Some of the other addresses that require H occur when P is still live.
>
> - LRA therefore has to spill P.
>
> - When it reallocates P, LRA chooses to use FRAME_POINTER_REGNUM,
>    which has been eliminated to the stack pointer.  (This is ok,
>    since the frame register is free.)
>
> - Spilling P causes LRA to reprocess the instruction that uses P.
>
> - When reprocessing the address that has P as its base, LRA first
>    applies the new allocation, to get FRAME_POINTER_REGNUM,
>    and then applies the elimination, to get the stack pointer.
>
> The last step seems wrong: the elimination should only apply to
> pre-existing uses of FRAME_POINTER_REGNUM, not to uses that result
> from allocating pseudos.  Applying both means that we get the wrong
> register number, and therefore the wrong class.
>
> The PR is about an existing testcase that fails with LRA on m86k.
>
> Tested on aarch64-linux-gnu, powerpc64le-linux-gnu, and
> x86_64-linux-gnu.  Also tested by building at least one target
> per CPU directory and checking that there were no asm changes or
> new ICEs when compiling gcc.c-torture, gcc.dg, and g++.dg at -Os.
> OK to install?
Yes, Richard.  Thank you for solving this PR, thorough testing,  and 
high-level description of the problem.
> gcc/
> 	PR middle-end/116321
> 	* lra-constraints.cc (get_hard_regno): Only apply eliminations
> 	to existing hard registers.
> 	(get_reg_class): Likewise.
diff mbox series

Patch

diff --git a/gcc/lra-constraints.cc b/gcc/lra-constraints.cc
index 90cbe6c012b..fdcc07764a2 100644
--- a/gcc/lra-constraints.cc
+++ b/gcc/lra-constraints.cc
@@ -200,12 +200,13 @@  get_hard_regno (rtx x)
     reg = SUBREG_REG (x);
   if (! REG_P (reg))
     return -1;
-  if (! HARD_REGISTER_NUM_P (hard_regno = REGNO (reg)))
-    hard_regno = lra_get_regno_hard_regno (hard_regno);
+  int regno = REGNO (reg);
+  if (HARD_REGISTER_NUM_P (regno))
+    hard_regno = lra_get_elimination_hard_regno (regno);
+  else
+    hard_regno = lra_get_regno_hard_regno (regno);
   if (hard_regno < 0)
     return -1;
-  if (HARD_REGISTER_NUM_P (REGNO (reg)))
-    hard_regno = lra_get_elimination_hard_regno (hard_regno);
   if (SUBREG_P (x))
     hard_regno += subreg_regno_offset (hard_regno, GET_MODE (reg),
 				       SUBREG_BYTE (x),  GET_MODE (x));
@@ -221,13 +222,12 @@  get_reg_class (int regno)
 {
   int hard_regno;
 
-  if (! HARD_REGISTER_NUM_P (hard_regno = regno))
+  if (HARD_REGISTER_NUM_P (regno))
+    hard_regno = lra_get_elimination_hard_regno (regno);
+  else
     hard_regno = lra_get_regno_hard_regno (regno);
   if (hard_regno >= 0)
-    {
-      hard_regno = lra_get_elimination_hard_regno (hard_regno);
-      return REGNO_REG_CLASS (hard_regno);
-    }
+    return REGNO_REG_CLASS (hard_regno);
   if (regno >= new_regno_start)
     return lra_get_allocno_class (regno);
   return NO_REGS;