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.ccindex 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. */
