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))
{
