[PATCH] D22420: Fix check for prescheduling to copies from virtual registers

[Elliot Colp via llvm-commits]

Hmm, you're right -- seems like I misread that line. Looking at this with
fresh eyes, I agree that this change doesn't look correct.

This patch was an attempt to fix an error related to DAG scheduling when
compiling for SystemZ, but evidently this isn't the right way to do it.
Who would be best to talk to about this part of the code so I can get a
better idea of what's going wrong?

Justin Bogner <justin at justinbogner.com> wrote on 2016/07/16 02:54:24 PM:

> From: Justin Bogner <mail at justinbogner.com>
> To: Elliot Colp/Toronto/IBM at IBMCA
> Cc: reviews+D22420+public+c33f2e5d5d477fac at reviews.llvm.org,
> matze at braunis.de, llvm-commits at lists.llvm.org
> Date: 2016/07/16 02:54 PM
> Subject: Re: [PATCH] D22420: Fix check for prescheduling to copies
> from virtual registers
> Sent by: Justin Bogner <justin at justinbogner.com>
>
> Elliot Colp <colpell at ca.ibm.com> writes:
> > colpell created this revision.
> > colpell added a reviewer: bogner.
> > colpell added a subscriber: llvm-commits.
> > Herald added a subscriber: MatzeB.
> >
> > Currently, this check on line 2834 is an exact copy of a check on line
> > 2810, which SU should have already passed.
>
> That's not entirely true. They both check SU.getNode(), sure, but one
> checks if the Opcode is CopyToReg and the other if it's CopyFromReg.
> This change definitely changes behaviour and needs to come with a test
> if it's correct, which I'm not really convinced it is.
>
> > Given the context and comment above the check ("Avoid prescheduling
> > *to* copies" vs. "Avoid prescheduling copies"), it seems like the
> > intention was to check PredSU instead.
>
> > https://reviews.llvm.org/D22420
> >
> > Files:
> >   lib/CodeGen/SelectionDAG/ScheduleDAGRRList.cpp
> >
> > Index: lib/CodeGen/SelectionDAG/ScheduleDAGRRList.cpp
> > ===================================================================
> > --- lib/CodeGen/SelectionDAG/ScheduleDAGRRList.cpp
> > +++ lib/CodeGen/SelectionDAG/ScheduleDAGRRList.cpp
> > @@ -2831,7 +2831,7 @@
> >        continue;
> >      // Avoid prescheduling to copies from virtual registers,
> which don't behave
> >      // like other nodes from the perspective of scheduling heuristics.
> > -    if (SDNode *N = SU.getNode())
> > +    if (SDNode *N = PredSU->getNode())
> >        if (N->getOpcode() == ISD::CopyFromReg &&
> >            TargetRegisterInfo::isVirtualRegister
> >              (cast<RegisterSDNode>(N->getOperand(1))->getReg()))
> >
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20160719/31fbb4c0/attachment.html>