On 6/19/25 7:43 AM, Alexandre Oliva wrote:
If the frame size grows to nonzero, arm_frame_pointer_required may
flip to true under -fstack-clash-protection -fnon-call-exceptions, and
that may disable the fp2sp elimination part-way through lra.
If pseudos had got assigned to the frame pointer register before that,
they have to be spilled, and that requires complete live range
information. If !lra_reg_spill_p, lra_spill won't have live ranges
for such pseudos, and they could end up sharing spill slots with other
pseudos whose live ranges actually overlap.
This affects at least Ada.Strings.Wide_Superbounded.Super_Insert and
.Super_Replace_Slice in libgnat/a-stwisu.adb, when compiled with -O2
-fstack-clash-protection -march=armv7 (implied Thumb2), causing
acats-4's cdd2a01 to fail.
Recomputing live ranges including registers may renumber and compress
points, so we have to recompute the aggregated live ranges for
already-assigned spill slots as well.
As a safety net, reject empty live ranges when computing slot sharing.
Regstrapped on x86_64-linux-gnu, and regstrapping on arm-linux-gnueabihf
in modes arm and thumb, with -fstack-clash-protection
-fnon-call-exceptions in BOOT_CFLAGS and TFLAGS. Also tested with
gcc-14 acats-4 on arm-vx7r2 and arm-linux-gnueabihf, with both options
enabled for Ada. Ok to install?
Alternative solution could be always building live ranges for pseudos assigned
to the frame pointer. But I think your solution is good enough as it is a rare
event when the frame pointer becomes not eliminable.
lra_complete_live_ranges has a parameter which is not necessary as it always
has the same value. It would be nice to remove it.
lra_create_live_ranges in lra_complete_live_ranges could pass false
(dead_insn_p). But there is no harm to keep it true. So let it stay.
I like that additional asserts about our assumptions are added.
So the patch with removing the param for lra_complete_live_ranges is ok for me.
Thank you for working on this PR, Alex.
for gcc/ChangeLog
PR rtl-optimization/120424
* lra-eliminations.cc (lra_update_fp2sp_elimination):
Compute complete live ranges and recompute slots' live ranges
if needed.
* lra-lives.cc (lra_reset_live_range_list): New.
(lra_complete_live_ranges): New.
* lra-spills.cc (assign_spill_hard_regs): Reject empty live
ranges.
(add_pseudo_to_slot): Likewise.
(lra_recompute_slots_live_ranges): New.
* lra-int.h (lra_reset_live_range_list): Declare.
(lra_complete_live_ranges): Declare.
(lra_recompute_slots_live_ranges): Declare.
---
gcc/lra-eliminations.cc | 8 ++++++++
gcc/lra-int.h | 3 +++
gcc/lra-lives.cc | 22 ++++++++++++++++++++++
gcc/lra-spills.cc | 37 +++++++++++++++++++++++++++++++++++++
4 files changed, 70 insertions(+)
diff --git a/gcc/lra-eliminations.cc b/gcc/lra-eliminations.cc
index 6c8c91086f323..09959bbe3ed9c 100644
--- a/gcc/lra-eliminations.cc
+++ b/gcc/lra-eliminations.cc
@@ -1429,6 +1429,14 @@ lra_update_fp2sp_elimination (int *spilled_pseudos)
frame_pointer_needed = true;
CLEAR_HARD_REG_SET (set);
add_to_hard_reg_set (&set, Pmode, HARD_FRAME_POINTER_REGNUM);
+ /* If !lra_reg_spill_p, we likely have incomplete range information
+ for pseudos assigned to the frame pointer that will have to be
+ spilled, and so we may end up incorrectly sharing them unless we
+ get live range information for them. */
+ if (lra_complete_live_ranges (true))
+ /* If lives ranges changed, update the aggregate live ranges in
+ slots as well before spilling any further pseudos. */
+ lra_recompute_slots_live_ranges ();
n = spill_pseudos (set, spilled_pseudos);
if (!ep)
for (ep = reg_eliminate; ep < ®_eliminate[NUM_ELIMINABLE_REGS]; ep++)
diff --git a/gcc/lra-int.h b/gcc/lra-int.h
index ad42f48cc822f..99e4699648cc9 100644
--- a/gcc/lra-int.h
+++ b/gcc/lra-int.h
@@ -381,7 +381,9 @@ extern int *lra_point_freq;
extern int lra_hard_reg_usage[FIRST_PSEUDO_REGISTER];
extern int lra_live_range_iter;
+extern void lra_reset_live_range_list (lra_live_range_t &);
extern void lra_create_live_ranges (bool, bool);
+extern bool lra_complete_live_ranges (bool);
extern lra_live_range_t lra_copy_live_range_list (lra_live_range_t);
extern lra_live_range_t lra_merge_live_ranges (lra_live_range_t,
lra_live_range_t);
@@ -417,6 +419,7 @@ extern bool lra_need_for_scratch_reg_p (void);
extern bool lra_need_for_spills_p (void);
extern void lra_spill (void);
extern void lra_final_code_change (void);
+extern void lra_recompute_slots_live_ranges (void);
/* lra-remat.cc: */
diff --git a/gcc/lra-lives.cc b/gcc/lra-lives.cc
index ffce162907e93..722efc83bbc49 100644
--- a/gcc/lra-lives.cc
+++ b/gcc/lra-lives.cc
@@ -113,6 +113,15 @@ free_live_range_list (lra_live_range_t lr)
}
}
+/* Reset and release live range list LR. */
+void
+lra_reset_live_range_list (lra_live_range_t &lr)
+{
+ lra_live_range_t first = lr;
+ lr = NULL;
+ free_live_range_list (first);
+}
+
/* Create and return pseudo live range with given attributes. */
static lra_live_range_t
create_live_range (int regno, int start, int finish, lra_live_range_t next)
@@ -1524,6 +1533,19 @@ lra_create_live_ranges (bool all_p, bool dead_insn_p)
lra_assert (! res);
}
+/* Run lra_create_live_ranges if !complete_info_p. Return FALSE iff
+ live ranges are known to have remained unchanged. */
+
+bool
+lra_complete_live_ranges (bool dead_insn_p)
+{
+ if (complete_info_p)
+ return false;
+
+ lra_create_live_ranges (true, dead_insn_p);
+ return true;
+}
+
/* Finish all live ranges. */
void
lra_clear_live_ranges (void)
diff --git a/gcc/lra-spills.cc b/gcc/lra-spills.cc
index 4febc693d2838..7603d0dcf163a 100644
--- a/gcc/lra-spills.cc
+++ b/gcc/lra-spills.cc
@@ -261,6 +261,7 @@ assign_spill_hard_regs (int *pseudo_regnos, int n)
for (res = i = 0; i < n; i++)
{
regno = pseudo_regnos[i];
+ gcc_assert (lra_reg_info[regno].live_ranges);
rclass = lra_get_allocno_class (regno);
if (bitmap_bit_p (setjump_crosses, regno)
|| (spill_class
@@ -346,15 +347,51 @@ add_pseudo_to_slot (int regno, int slot_num)
first = pseudo_slots[regno].first =
&pseudo_slots[slots[slot_num].regno];
pseudo_slots[regno].next = first->next;
first->next = &pseudo_slots[regno];
+ lra_assert (slots[slot_num].live_ranges);
}
pseudo_slots[regno].mem = NULL_RTX;
pseudo_slots[regno].slot_num = slot_num;
+
+ /* Pseudos with empty ranges shouldn't need to be spilled; if we get
+ an empty range added to a slot, something fishy is going on, such
+ as missing live range information, and without that information
+ we may generate wrong code. We can probably relax this to an
+ lra_assert at some point. Likewise the analogous one in
+ assign_spill_hard_regs. */
+ gcc_assert (lra_reg_info[regno].live_ranges);
+
slots[slot_num].live_ranges
= lra_merge_live_ranges (slots[slot_num].live_ranges,
lra_copy_live_range_list
(lra_reg_info[regno].live_ranges));
}
+/* Recompute the combined live ranges of pseudos assigned to stack
+ slots. This brings the live ranges of slots back in sync with
+ those of pseudos, after recomputing live ranges for pseudos. */
+void
+lra_recompute_slots_live_ranges (void)
+{
+ for (int i = 0; i < slots_num; i++)
+ {
+ if (slots[i].regno < 0)
+ continue;
+ lra_reset_live_range_list (slots[i].live_ranges);
+ for (struct pseudo_slot *next = pseudo_slots[slots[i].regno].first;
+ next; next = next->next)
+ {
+ int regno = next - pseudo_slots;
+ lra_assert (!(lra_intersected_live_ranges_p
+ (lra_reg_info[regno].live_ranges,
+ slots[i].live_ranges)));
+ slots[i].live_ranges
+ = lra_merge_live_ranges (slots[i].live_ranges,
+ lra_copy_live_range_list
+ (lra_reg_info[regno].live_ranges));
+ }
+ }
+}
+
/* Assign stack slot numbers to pseudos in array PSEUDO_REGNOS of
length N. Sort pseudos in PSEUDO_REGNOS for subsequent assigning
memory stack slots. */