This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: [RS6000] Fix PowerPC ICE due to secondary_reload ignoring reload replacements
- From: Alan Modra <amodra at gmail dot com>
- To: David Edelsohn <dje dot gcc at gmail dot com>, Ulrich Weigand <uweigand at de dot ibm dot com>
- Cc: GCC Patches <gcc-patches at gcc dot gnu dot org>
- Date: Thu, 10 Sep 2015 10:27:55 +0930
- Subject: Re: [RS6000] Fix PowerPC ICE due to secondary_reload ignoring reload replacements
- Authentication-results: sourceware.org; auth=none
- References: <CAGWvnyk7ZYYBbj0RNg=YhdtZdRnKqhYEgZBymyENhW+99nro4A at mail dot gmail dot com> <20150908124658 dot 8E0D523EC at oc7340732750 dot ibm dot com>
On Tue, Sep 08, 2015 at 02:46:58PM +0200, Ulrich Weigand wrote:
> David Edelsohn wrote:
> > On Mon, Sep 7, 2015 at 11:47 PM, Alan Modra <amodra@gmail.com> wrote:
> > > In https://gcc.gnu.org/bugzilla/show_bug.cgi?id=67378 analysis I show
> > > the reason for this PR is that insns emitted by secondary reload
> > > patterns are being generated without taking into account other reloads
> > > that may have occurred. We run into this problem when an insn has a
> > > pseudo that doesn't get a hard reg, and the pseudo is used in a way
> > > that requires a secondary reload. In this case the secondary reload
> > > is needed due to gcc generating a 64-bit gpr load from memory insn
> > > with an address offset not a multiple of 4.
> > >
> > > PR target/67378
> > > * config/rs6000/rs6000.c (rs6000_secondary_reload_gpr): Find
> > > reload replacement for PRE_MODIFY address reg.
> >
> > I'm okay with this patch, but I'd like Uli to double-check it when he
> > has a moment.
>
> The patch looks OK to me. We definitely need to check for replacements
> in secondary reload in such cases.
Thanks for reviewing! I think rs6000_secondary_reload_inner needs the
same. (https://gcc.gnu.org/ml/gcc-patches/2014-11/msg01114.html
removed a bunch of find_replacement calls that I'd added previously.)
This patch reinstates some of the calls, a little more elegantly than
in my original effort. I've also corrected an obvious error with the
PRE_DEC address offset. Bootstrapped and regression tested
powerpc64le-linux. OK for mainline and gcc-5?
* config/rs6000/rs6000.c (rs6000_secondary_reload_inner): Find
reload replacement for PRE_INC/DEC, PRE_MODIFY and AND address regs.
Negate offset for PRE_DEC.
diff --git a/gcc/config/rs6000/rs6000.c b/gcc/config/rs6000/rs6000.c
index b9f35cd..f616b21 100644
--- a/gcc/config/rs6000/rs6000.c
+++ b/gcc/config/rs6000/rs6000.c
@@ -18020,7 +18020,12 @@ rs6000_secondary_reload_inner (rtx reg, rtx mem, rtx scratch, bool store_p)
if ((addr_mask & RELOAD_REG_PRE_INCDEC) == 0)
{
- emit_insn (gen_add2_insn (op_reg, GEN_INT (GET_MODE_SIZE (mode))));
+ if (!HARD_REGISTER_P (op_reg))
+ op_reg = find_replacement (&XEXP (addr, 0));
+ int delta = GET_MODE_SIZE (mode);
+ if (GET_CODE (addr) == PRE_DEC)
+ delta = -delta;
+ emit_insn (gen_add2_insn (op_reg, GEN_INT (delta)));
new_addr = op_reg;
}
break;
@@ -18035,6 +18040,8 @@ rs6000_secondary_reload_inner (rtx reg, rtx mem, rtx scratch, bool store_p)
if ((addr_mask & RELOAD_REG_PRE_MODIFY) == 0)
{
+ if (!HARD_REGISTER_P (op0))
+ op0 = find_replacement (&XEXP (addr, 0));
emit_insn (gen_rtx_SET (op0, op1));
new_addr = reg;
}
@@ -18048,7 +18055,11 @@ rs6000_secondary_reload_inner (rtx reg, rtx mem, rtx scratch, bool store_p)
if ((addr_mask & RELOAD_REG_AND_M16) == 0)
{
if (REG_P (op0) || GET_CODE (op0) == SUBREG)
- op_reg = op0;
+ {
+ op_reg = op0;
+ if (!HARD_REGISTER_P (op_reg))
+ op_reg = find_replacement (&XEXP (addr, 0));
+ }
else if (GET_CODE (op1) == PLUS)
{
--
Alan Modra
Australia Development Lab, IBM