This is part of making m68k work with LRA.  See PR116429.
In short: setup_sp_offset is internally inconsistent.  It wants to
setup the sp_offset for newly generated instructions.  sp_offset for
an instruction is always the state of the sp-offset right before that
instruction.  For that it starts at the (assumed correct) sp_offset
of the instruction right after the given (new) sequence, and then
iterates that sequence forward simulating its effects on sp_offset.

That can't ever be right: either it needs to start at the front
and simulate forward, or start at the end and simulate backward.
The former seems to be the more natural way.  Funnily the local
variable holding that instruction is also called 'before'.

This changes it to the first variant: start before the sequence,
do one simulation step to get the sp-offset state in front of the
sequence and then continue simulating.

More details: in the problematic testcase we start with this
situation (sp_off before 550 is 0):

  550: [--sp] = 0             sp_off = 0  {pushexthisi_const}
  551: [--sp] = 37            sp_off = -4 {pushexthisi_const}
  552: [--sp] = r37           sp_off = -8 {movsi_m68k2}
  554: [--sp] = r116 - r37    sp_off = -12 {subsi3}
  556: call                   sp_off = -16

insn 554 doesn't match its constraints and needs some reloads:

      Creating newreg=262, assigning class DATA_REGS to r262
  554: r262:SI=r262:SI-r37:SI
      REG_ARGS_SIZE 0x10
    Inserting insn reload before:
  996: r262:SI=r116:SI
    Inserting insn reload after:
  997: [--%sp:SI]=r262:SI

         Considering alt=0 of insn 997:   (0) =g  (1) damSKT
            1 Non pseudo reload: reject++
          overall=1,losers=0,rld_nregs=0
      Choosing alt 0 in insn 997:  (0) =g  (1) damSKT {*movsi_m68k2} 
(sp_off=-16)

Note how insn 997 (the after-reload) now has sp_off=-16 already.  It all
goes downhill from there.  We end up with these insns:

  552: [--sp] = r37           sp_off = -8 {movsi_m68k2}
  996: r262 = r116            sp_off = -12
  554: r262 = r262 - r37      sp_off = -12
  997: [--sp] = r262          sp_off = -16  (!!! should be -12)
  556: call                   sp_off = -16

The call insn sp_off remains at the correct -16, but internally it's already
inconsistent here.  If the sp_off before an insn is -16, and that insn
pre_decs sp, then the after-insn sp_off should be -20.

        PR target/116429
        * lra.cc (setup_sp_offset): Start with sp_offset from
        before the new sequence, not from after.
---
 gcc/lra.cc | 13 ++++++++-----
 1 file changed, 8 insertions(+), 5 deletions(-)
---
Regstrapped on x86-64-linux.  Okay?


diff --git a/gcc/lra.cc b/gcc/lra.cc
index fb32e134004..b84384b2145 100644
--- a/gcc/lra.cc
+++ b/gcc/lra.cc
@@ -1863,14 +1863,17 @@ push_insns (rtx_insn *from, rtx_insn *to)
 }
 
 /* Set up and return sp offset for insns in range [FROM, LAST].  The offset is
-   taken from the next BB insn after LAST or zero if there in such
-   insn.  */
+   taken from the BB insn before FROM after simulating its effects,
+   or zero if there is no such insn.  */
 static poly_int64
 setup_sp_offset (rtx_insn *from, rtx_insn *last)
 {
-  rtx_insn *before = next_nonnote_nondebug_insn_bb (last);
-  poly_int64 offset = (before == NULL_RTX || ! INSN_P (before)
-                      ? 0 : lra_get_insn_recog_data (before)->sp_offset);
+  rtx_insn *before = prev_nonnote_nondebug_insn_bb (from);
+  poly_int64 offset = 0;
+
+  if (before && INSN_P (before))
+    offset = lra_update_sp_offset (PATTERN (before),
+                                  lra_get_insn_recog_data (before)->sp_offset);
 
   for (rtx_insn *insn = from; insn != NEXT_INSN (last); insn = NEXT_INSN 
(insn))
     {
-- 
2.39.1

Reply via email to