Hi Bill,
On Tue, Aug 30, 2016 at 08:23:46PM -0500, Bill Schmidt wrote:
> The ada bootstrap failure reported in
> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=72827
> occurs because of a latent bug in the powerpc back end. The immediate cause
> is dead store
> elimination removing two stores relative to the frame pointer that are not
> dead; however,
> DSE is tricked into doing this because we have temporarily adjusted the frame
> pointer prior
> to performing the loads. DSE relies on the frame pointer remaining constant
> to be able to
> infer stack stores that are dead.
DSE should really detect this is happening and not do the wrong thing.
Maybe add an assert somewhere? Much easier to debug, that way.
> Is this ok for trunk, and eventually for backport to the 5 and 6 branches?
Will it backport without changes?
> Index: gcc/config/rs6000/rs6000.c
> ===================================================================
> --- gcc/config/rs6000/rs6000.c (revision 239871)
> +++ gcc/config/rs6000/rs6000.c (working copy)
> @@ -24506,15 +24506,31 @@ rs6000_split_multireg_move (rtx dst, rtx src)
> && REG_P (basereg)
> && REG_P (offsetreg)
> && REGNO (basereg) != REGNO (offsetreg));
> - if (REGNO (basereg) == 0)
> + /* We mustn't modify the stack pointer or frame pointer
> + as this will confuse dead store elimination. */
> + if ((REGNO (basereg) == STACK_POINTER_REGNUM
> + || REGNO (basereg) == HARD_FRAME_POINTER_REGNUM)
> + && REGNO (offsetreg) != 0)
This should only check for HARD_FRAME_POINTER_REGNUM if there *is* a
frame pointer?
> {
> - rtx tmp = offsetreg;
> - offsetreg = basereg;
> - basereg = tmp;
std::swap (basereg, offsetreg);
> + emit_insn (gen_add3_insn (offsetreg, basereg,
> + offsetreg));
> + restore_basereg = gen_sub3_insn (offsetreg, offsetreg,
> + basereg);
> + dst = replace_equiv_address (dst, offsetreg);
> }
> - emit_insn (gen_add3_insn (basereg, basereg, offsetreg));
> - restore_basereg = gen_sub3_insn (basereg, basereg, offsetreg);
> - dst = replace_equiv_address (dst, basereg);
> + else
> + {
> + if (REGNO (basereg) == 0)
> + {
> + rtx tmp = offsetreg;
> + offsetreg = basereg;
> + basereg = tmp;
> + }
> + emit_insn (gen_add3_insn (basereg, basereg, offsetreg));
> + restore_basereg = gen_sub3_insn (basereg, basereg,
> + offsetreg);
> + dst = replace_equiv_address (dst, basereg);
> + }
> }
> }
> else if (GET_CODE (XEXP (dst, 0)) != LO_SUM)
If (say) base=r1 offset=r0 this will now adjust r1? That cannot be good.
Segher
