On 6/23/25 12:06 AM, Alexandre Oliva wrote:
Alex, thanks for investigation of corner cases of register elimination.
An x86_64-linux-gnu native with ix86_frame_pointer_required modified
to return true for nonzero frames, to exercize
lra_update_fp2sp_elimination, reveals in stage1 testing that wrong
code is generated for gcc.c-torture/execute/ieee/fp-cmp-8l.c:
argp-to-sp eliminations are used for one_test to pass its arguments on
to *pos, and the sp offsets survive the disabling of that elimination.
We didn't really have to disable that elimination, but the backend
disables eliminations to sp if frame_pointer_needed.
The workaround for this scenario is to compile with
-maccumulate-outgoing-args.
This change extends the catching of fp2sp eliminations to all (?)
eliminations to sp, since none of them can be properly reversed and
would silently lead to wrong code. This is probably too strict.
I guess it is too strict.
Regstrapped on x86_64-linux-gnu, bootstrapped on arm-linux-gnueabihf
(arm and thumb modes), also tested with gcc-14 on arm-vx7r2 and
arm-linux-gnueabihf.
Unlike the combination of earlier patches, this one does NOT bootstrap
on x86_64-linux-gnu with ix86_frame_pointer_required modified to return
true for any positive frame sizes.
It also triggers one failure in acats-4 on arm-vx7r2, where I didn't
expect it to make any difference. I'm yet to investigate it.
I wonder if it makes sense to put this in to (1.i) avoid silent wrong
code, and (1.ii) shake out some more lra_update_fp2sp_elimination
issues, or (2) keep it out and just file a PR about this one known
remaining issue, AFAICT only fixable by making sp offset adjustments
reversible. WDYT?
I am not ready to answer the question about committing the patch right
now. It needs more time for investigation which I currently don't have
but will have when the next release work starts.
In general I think we should have functionality generating the right
code whenever any elimination goes prohibited or enabled back and forth
during LRA work. That is what I would like to aim at. It might require
considerable review of all existing elimination code.
So I think the best way right now is to fill a PR.
for gcc/ChangeLog
PR rtl-optimization/120424
* lra-eliminations.cc (elimination_2sp_occurred_p): Rename
from...
(elimination_fp2sp_occured_p): ... this. Adjust all uses.
(lra_eliminate_regs_1): Don't require a from-frame-pointer
elimination to set it.
(update_reg_eliminate): Likewise to test it.
---
gcc/lra-eliminations.cc | 24 ++++++++++++------------
1 file changed, 12 insertions(+), 12 deletions(-)
diff --git a/gcc/lra-eliminations.cc b/gcc/lra-eliminations.cc
index 9cdd0c5ff53a2..f6ee33aa70a5d 100644
--- a/gcc/lra-eliminations.cc
+++ b/gcc/lra-eliminations.cc
@@ -309,8 +309,9 @@ move_plus_up (rtx x)
return x;
}
-/* Flag that we already did frame pointer to stack pointer elimination. */
-static bool elimination_fp2sp_occured_p = false;
+/* Flag that we already applied stack pointer elimination offset; sp
+ updates cannot be undone. */
+static bool elimination_2sp_occurred_p = false;
/* Scan X and replace any eliminable registers (such as fp) with a
replacement (such as sp) if SUBST_P, plus an offset. The offset is
@@ -369,8 +370,8 @@ lra_eliminate_regs_1 (rtx_insn *insn, rtx x, machine_mode
mem_mode,
{
rtx to = subst_p ? ep->to_rtx : ep->from_rtx;
- if (ep->to_rtx == stack_pointer_rtx && ep->from == FRAME_POINTER_REGNUM)
- elimination_fp2sp_occured_p = true;
+ if (ep->to_rtx == stack_pointer_rtx)
+ elimination_2sp_occurred_p = true;
if (maybe_ne (update_sp_offset, 0))
{
@@ -402,8 +403,8 @@ lra_eliminate_regs_1 (rtx_insn *insn, rtx x, machine_mode
mem_mode,
poly_int64 offset, curr_offset;
rtx to = subst_p ? ep->to_rtx : ep->from_rtx;
- if (ep->to_rtx == stack_pointer_rtx && ep->from == FRAME_POINTER_REGNUM)
- elimination_fp2sp_occured_p = true;
+ if (ep->to_rtx == stack_pointer_rtx)
+ elimination_2sp_occurred_p = true;
if (! update_p && ! full_p)
return simplify_gen_binary (PLUS, Pmode, to, XEXP (x, 1));
@@ -465,8 +466,8 @@ lra_eliminate_regs_1 (rtx_insn *insn, rtx x, machine_mode
mem_mode,
{
rtx to = subst_p ? ep->to_rtx : ep->from_rtx;
- if (ep->to_rtx == stack_pointer_rtx && ep->from == FRAME_POINTER_REGNUM)
- elimination_fp2sp_occured_p = true;
+ if (ep->to_rtx == stack_pointer_rtx)
+ elimination_2sp_occurred_p = true;
if (maybe_ne (update_sp_offset, 0))
{
@@ -1213,8 +1214,7 @@ update_reg_eliminate (bitmap insns_with_changed_offsets)
pointer elimination the condition is a bit relaxed and we just
require
that actual elimination has not been done yet. */
gcc_assert (ep->to_rtx != stack_pointer_rtx
- || (ep->from == FRAME_POINTER_REGNUM
- && !elimination_fp2sp_occured_p)
+ || !elimination_2sp_occurred_p
|| (ep->from < FIRST_PSEUDO_REGISTER
&& fixed_regs [ep->from]));
@@ -1429,7 +1429,7 @@ lra_update_fp2sp_elimination (int *spilled_pseudos)
if (frame_pointer_needed || !targetm.frame_pointer_required ())
return 0;
- gcc_assert (!elimination_fp2sp_occured_p);
+ gcc_assert (!elimination_2sp_occurred_p);
ep = elimination_map[FRAME_POINTER_REGNUM];
if (ep->to == STACK_POINTER_REGNUM)
{
@@ -1499,7 +1499,7 @@ lra_eliminate (bool final_p, bool first_p)
if (first_p)
{
- elimination_fp2sp_occured_p = false;
+ elimination_2sp_occurred_p = false;
init_elimination ();
}
