Re: [pushed][LRA]: Spill pseudos assigned to fp when fp->sp elimination became impossible

[SenthilKumar.Selvaraj--- via Gcc-patches]

On Wed, 2023-08-16 at 12:13 -0400, Vladimir Makarov wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you know the 
> content is safe
> 
> The attached patch fixes recently found wrong insn removal in LRA port
> for AVR.
> 
> The patch was successfully tested and bootstrapped on x86-64 and aarch64.
> 
> 
Hi Vladimir,

  Thanks for working on this. After applying the patch, I'm seeing that the
  pseudo in the frame pointer that got spilled is taking up the same stack
  slot that was already assigned to a spilled pseudo, and that is causing 
execution
  failure (it is also causing a crash when building libgcc for avr)

(insn 108 15 3 3 (set (mem/c:SI (plus:HI (reg/f:HI 28 r28)
                (const_int 1 [0x1])) [3 %sfp+1 S4 A8])
        (reg:SI 4 r4 [orig:46 f.3_4 ] [46])) 
"/home/i41766/code/personal/gcc/gcc/testsuite/gcc.c-torture/execute/20050224-
1.c":29:16 119 {*movsi_split}
     (nil))
(insn 3 108 4 3 (set (mem/c:HI (plus:HI (reg/f:HI 28 r28)
                (const_int 1 [0x1])) [3 %sfp+1 S2 A8])
        (const_int 0 [0])) 
"/home/i41766/code/personal/gcc/gcc/testsuite/gcc.c-torture/execute/20050224-1.c":19:21
 101
{*movhi_split}
     (expr_list:REG_EQUAL (const_int 0 [0])
        (nil)))

  Both pseudo 46, and the pseudo spilled off FP (51) get assigned stack slot 0.
  This translates to this obviously wrong assembly code.

        lds r4,f
        lds r5,f+1
        lds r6,f+2
        lds r7,f+3
        std Y+1,r4
        std Y+2,r5
        std Y+3,r6
        std Y+4,r7
        std Y+2,__zero_reg__
        std Y+1,__zero_reg__

  I tried a hacky workaround (see patch below) to create a new stack slot and
  assign the spilled pseudo to it, and that works.
  
  Not sure if that's the right way to do it though.

Regards
Senthil

  diff --git gcc/lra-spills.cc gcc/lra-spills.cc
index 7e1d35b5e4e..e985ab56a60 100644
--- gcc/lra-spills.cc
+++ gcc/lra-spills.cc
@@ -359,11 +359,12 @@ add_pseudo_to_slot (int regno, int slot_num)
    length N.  Sort pseudos in PSEUDO_REGNOS for subsequent assigning
    memory stack slots. */
 static void
-assign_stack_slot_num_and_sort_pseudos (int *pseudo_regnos, int n)
+assign_stack_slot_num_and_sort_pseudos (int *pseudo_regnos, int n, bool reset)
 {
   int i, j, regno;
 
-  slots_num = 0;
+  if (reset)
+    slots_num = 0;
   /* Assign stack slot numbers to spilled pseudos, use smaller numbers
      for most frequently used pseudos. */
   for (i = 0; i < n; i++)
@@ -628,14 +629,15 @@ lra_spill (void)
   /* Sort regnos according their usage frequencies.  */
   qsort (pseudo_regnos, n, sizeof (int), regno_freq_compare);
   n = assign_spill_hard_regs (pseudo_regnos, n);
-  assign_stack_slot_num_and_sort_pseudos (pseudo_regnos, n);
+  assign_stack_slot_num_and_sort_pseudos (pseudo_regnos, n, true);
   for (i = 0; i < n; i++)
     if (pseudo_slots[pseudo_regnos[i]].mem == NULL_RTX)
       assign_mem_slot (pseudo_regnos[i]);
-  if ((n2 = lra_update_fp2sp_elimination (pseudo_regnos)) > 0)
+  if ((n2 = lra_update_fp2sp_elimination (&pseudo_regnos[n])) > 0)
     {
+      assign_stack_slot_num_and_sort_pseudos (&pseudo_regnos[n], n2, false);
       /* Assign stack slots to spilled pseudos assigned to fp.  */
-      for (i = 0; i < n2; i++)
+      for (i = n; i < n + n2; i++)
        if (pseudo_slots[pseudo_regnos[i]].mem == NULL_RTX)
          assign_mem_slot (pseudo_regnos[i]);
     }

Regards
Senthil