On Wed, Sep 9, 2015 at 8:57 PM, Alan Modra <amo...@gmail.com> wrote: > 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 <amo...@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?
Alan, You and Mike need to get on the same page. I don't want ping-ponging patches where you add a check and Mike knowingly or unknowingly removes it, then you add it back. Ideally I want a testcase. Barring that, I want a comment at all of these points explaining why find_replacement is necessary. Thanks, David
