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 < &reg_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.        */



Reply via email to