https://gcc.gnu.org/bugzilla/show_bug.cgi?id=116429

Michael Matz <matz at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |matz at gcc dot gnu.org

--- Comment #1 from Michael Matz <matz at gcc dot gnu.org> ---
This is a problem in LRAs setup_sp_offset.  The per-insn sp_offset item is
supposed to reflect the sp_offset before the insn in question.  So, we
(correctly) start with this sequence of insns before LRA, and figure out
(again,
correctly) the sp_offsets noted:

we start at sp_off = 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

Now, insn 554 (subsi3) doesn't match constraints, the destination is a reg,
needs to match the first input.  So we generate 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 and insn is -16, and that insn
pre_decs sp, then the after-insn sp_off should be -20.

This is all because setup_sp_offset updates the sp_offset for new insns
incorrectly.  It tries to setup it for the whole (new) insn sequence in order,
but for mysterious reasons starts off with the given sp_offset from the
_next_ instruction after the last insn in the new seq.  That can't ever be
right.  The sp-offset simulation runs in a forward direction (via NEXT_INSN).
The only connection to sp_offset of the next insn after the sequence is that
the offset should end up being equal to that one after simulating the sequence.

(Note how the local varname for the startup point is called 'before' :) )

It's possible that sometime it is correct that the sp_off of the after-last
insn is the correct one to start with.  But then also the simulation of the
insn sequence needs to run backward, not forward.  Generally that probably
can only be decided if setup_sp_offset is given information about on which
of the borders of the sequence it can rely.  But as written it's internally
inconsistent.  Making it consistent as per below fixes this instance of
the problem.  I wouldn't be surprised if it introduces bugs elsewhere,
in which case setup_sp_offset really needs to be amended by more information.

diff --git a/gcc/lra.cc b/gcc/lra.cc
index fb32e134004..5afb21c3ea6 100644
--- a/gcc/lra.cc
+++ b/gcc/lra.cc
@@ -1868,9 +1868,13 @@ push_insns (rtx_insn *from, rtx_insn *to)
 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 = next_nonnote_nondebug_insn_bb (last);
+  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))
     {

Reply via email to