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