On Jan 17, 2014, at 4:03 PM, Andrew Trick <atrick at apple.com> wrote:> > On Jan 17, 2014, at 3:54 PM, Hal Finkel <hfinkel at anl.gov> wrote: > >> Andy, et al., >> >> In ScheduleDAGInstrs::buildSchedGraph, the code for handling stores has this: >> >> if (!ExitSU.isPred(SU)) >> // Push store's up a bit to avoid them getting in between cmp >> // and branches. >> ExitSU.addPred(SDep(SU, SDep::Artificial)); >> >> This code does not seem to be in any way specific to compares; and in any case, at least on the PPC A2, scheduling stores in between the compare and the branch would not be a bad thing (because the core is in order, and the compare has a 2-cycle latency, so if there is nothing else to do, a store would not be a bad thing to put there). >> >> Can you explain the motivation for this (why or for what it is bad), and what else it might do (aside from the commented cmp/branch pairing)? I'm wondering if we should make this target dependent. > > I don’t agree with the existing comment. It’s possible that somewhere, maybe in target specific code, we make use of the extra store->exit edge, but I can’t remember any reason for it now.Do you have another mechanism for encouraging macro fusion? Thanks, /jakob
----- Original Message -----> From: "Jakob Stoklund Olesen" <stoklund at 2pi.dk> > To: "Andy Trick" <atrick at apple.com> > Cc: "Hal Finkel" <hfinkel at anl.gov>, "LLVM Developers Mailing List" <llvmdev at cs.uiuc.edu> > Sent: Friday, January 17, 2014 6:16:25 PM > Subject: Re: [LLVMdev] Artificial deps and stores > > > On Jan 17, 2014, at 4:03 PM, Andrew Trick <atrick at apple.com> wrote: > > > > > On Jan 17, 2014, at 3:54 PM, Hal Finkel <hfinkel at anl.gov> wrote: > > > >> Andy, et al., > >> > >> In ScheduleDAGInstrs::buildSchedGraph, the code for handling > >> stores has this: > >> > >> if (!ExitSU.isPred(SU)) > >> // Push store's up a bit to avoid them getting in between > >> cmp > >> // and branches. > >> ExitSU.addPred(SDep(SU, SDep::Artificial)); > >> > >> This code does not seem to be in any way specific to compares; and > >> in any case, at least on the PPC A2, scheduling stores in between > >> the compare and the branch would not be a bad thing (because the > >> core is in order, and the compare has a 2-cycle latency, so if > >> there is nothing else to do, a store would not be a bad thing to > >> put there). > >> > >> Can you explain the motivation for this (why or for what it is > >> bad), and what else it might do (aside from the commented > >> cmp/branch pairing)? I'm wondering if we should make this target > >> dependent. > > > > I don’t agree with the existing comment. It’s possible that > > somewhere, maybe in target specific code, we make use of the extra > > store->exit edge, but I can’t remember any reason for it now. > > Do you have another mechanism for encouraging macro fusion?I think that TII->shouldScheduleAdjacent, which is used by MacroFusion in MachineScheduler.cpp is supposed to do this. -Hal> > Thanks, > /jakob > >-- Hal Finkel Assistant Computational Scientist Leadership Computing Facility Argonne National Laboratory
On Jan 17, 2014, at 4:16 PM, Jakob Stoklund Olesen <stoklund at 2pi.dk> wrote:> > On Jan 17, 2014, at 4:03 PM, Andrew Trick <atrick at apple.com> wrote: > >> >> On Jan 17, 2014, at 3:54 PM, Hal Finkel <hfinkel at anl.gov> wrote: >> >>> Andy, et al., >>> >>> In ScheduleDAGInstrs::buildSchedGraph, the code for handling stores has this: >>> >>> if (!ExitSU.isPred(SU)) >>> // Push store's up a bit to avoid them getting in between cmp >>> // and branches. >>> ExitSU.addPred(SDep(SU, SDep::Artificial)); >>> >>> This code does not seem to be in any way specific to compares; and in any case, at least on the PPC A2, scheduling stores in between the compare and the branch would not be a bad thing (because the core is in order, and the compare has a 2-cycle latency, so if there is nothing else to do, a store would not be a bad thing to put there). >>> >>> Can you explain the motivation for this (why or for what it is bad), and what else it might do (aside from the commented cmp/branch pairing)? I'm wondering if we should make this target dependent. >> >> I don’t agree with the existing comment. It’s possible that somewhere, maybe in target specific code, we make use of the extra store->exit edge, but I can’t remember any reason for it now. > > Do you have another mechanism for encouraging macro fusion?The MI scheduler does it now via a proper mechanism. However, for targets that haven’t switched over it could be a minor, temporary issue (the old mechanism shown above didn’t work well anyway). For the time being it is probably best to leave it under a target option with a FIXME to avoid perturbing non-PPC/x86 targets. -Andy -------------- next part -------------- An HTML attachment was scrubbed... URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20140117/e48291ea/attachment.html>
On Jan 17, 2014, at 4:36 PM, Andrew Trick <atrick at apple.com> wrote:> > On Jan 17, 2014, at 4:16 PM, Jakob Stoklund Olesen <stoklund at 2pi.dk> wrote: > >> >> On Jan 17, 2014, at 4:03 PM, Andrew Trick <atrick at apple.com> wrote: >> >>> >>> On Jan 17, 2014, at 3:54 PM, Hal Finkel <hfinkel at anl.gov> wrote: >>> >>>> Andy, et al., >>>> >>>> In ScheduleDAGInstrs::buildSchedGraph, the code for handling stores has this: >>>> >>>> if (!ExitSU.isPred(SU)) >>>> // Push store's up a bit to avoid them getting in between cmp >>>> // and branches. >>>> ExitSU.addPred(SDep(SU, SDep::Artificial)); >>>> >>>> This code does not seem to be in any way specific to compares; and in any case, at least on the PPC A2, scheduling stores in between the compare and the branch would not be a bad thing (because the core is in order, and the compare has a 2-cycle latency, so if there is nothing else to do, a store would not be a bad thing to put there). >>>> >>>> Can you explain the motivation for this (why or for what it is bad), and what else it might do (aside from the commented cmp/branch pairing)? I'm wondering if we should make this target dependent. >>> >>> I don’t agree with the existing comment. It’s possible that somewhere, maybe in target specific code, we make use of the extra store->exit edge, but I can’t remember any reason for it now. >> >> Do you have another mechanism for encouraging macro fusion? > > The MI scheduler does it now via a proper mechanism. However, for targets that haven’t switched over it could be a minor, temporary issue (the old mechanism shown above didn’t work well anyway). > > For the time being it is probably best to leave it under a target option with a FIXME to avoid perturbing non-PPC/x86 targets.Since it doesn’t work well anyway, I’d say just remove it. Thanks, /jakob -------------- next part -------------- An HTML attachment was scrubbed... URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20140117/078656a4/attachment.html>