[llvm] Preferential treatment of labels in MI sched DAG construction

[Sergei Larin]

Sure. Here is (hopefully) the final version :)

Sergei

---
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by
The Linux Foundation


> -----Original Message-----
> From: Bill Wendling [mailto:bwendling at apple.com]
> Sent: Monday, February 11, 2013 5:57 PM
> To: Andrew Trick
> Cc: Sergei Larin; llvm-commits at cs.uiuc.edu
> Subject: Re: [llvm] Preferential treatment of labels in MI sched DAG
> construction
> 
> 
> On Feb 11, 2013, at 3:51 PM, Andrew Trick <atrick at apple.com> wrote:
> 
> >
> > On Feb 11, 2013, at 1:50 PM, Sergei Larin <slarin at codeaurora.org>
> wrote:
> >
> >>
> >> Andy,
> >>
> >>
> >> Following our discussion below here is the proposed patch.
> >> I also looked at the comment language for the CanHandleTerminators:
> >>
> >>   /// The standard DAG builder does not normally include terminators
> >> as DAG
> >>   /// nodes because it does not create the necessary dependencies to
> >> prevent
> >>   /// reordering. A specialized scheduler can overide
> >>   /// TargetInstrInfo::isSchedulingBoundary then enable this flag to
> >> indicate
> >>   /// it has taken responsibility for scheduling the terminator
> correctly.
> >>   bool CanHandleTerminators;
> >>
> >> ...and it seems clear and all encompassing enough to cover the
> >> proposed change. If you would like to modify it, just let me know
> and
> >> I'll amend the patch. Thanks.
> >
> > Hi Sergei,
> >
> > I might have misunderstood, but to make the requirements explicit for
> targets that do not have custom terminator/label schedulers, I think we
> should have this:
> >
> > -    assert((!MI->isTerminator() || CanHandleTerminators) && !MI-
> >isLabel() &&
> > +    assert(CanHandleTerminators || (!MI->isTerminator() &&
> > + MI->isLabel()) &&
> >
> Don't forget the parens around the ||'ed stuff. :-)
> 
> -bw
> 
> > -Andy
> >
> >> ---
> >> Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
> >> hosted by The Linux Foundation
> >>
> >>
> >>> -----Original Message-----
> >>> From: Andrew Trick [mailto:atrick at apple.com]
> >>> Sent: Monday, February 11, 2013 3:23 PM
> >>> To: Sergei Larin
> >>> Cc: llvmdev at cs.uiuc.edu
> >>> Subject: Re: [LLVMdev] Preferential treatment of labels in MI sched
> >>> DAG construction
> >>>
> >>>
> >>> On Feb 11, 2013, at 1:03 PM, Sergei Larin <slarin at codeaurora.org>
> >>> wrote:
> >>>
> >>>> Hi Andy,
> >>>>
> >>>> I have to resurrect an ancient question regarding scheduling
> >>> boundaries.
> >>>>
> >>>> You might remember the reason for introduction of
> >>> CanHandleTerminators
> >>>> to ScheduleDAGInstrs. In short, Hexagon is currently uses DAG
> >>>> construction method (buildSchedGraph) for several purposes, which
> >>>> includes region formation for general VLIW packetization/bundling.
> >>>> As such we need to handle pretty much all instructions and any
> >>>> terminators, including labels (though I know it sounds strange).
> >>>> Nevertheless, ScheduleDAGInstrs::buildSchedGraph
> >>>> still has this hard assert in it:
> >>>>
> >>>> assert((!MI->isTerminator() || CanHandleTerminators) && !MI-
> >>>> isLabel() &&
> >>>>         "Cannot schedule terminators or labels!");
> >>>>
> >>>> ...and we have been OK till now by simply disabling it in our
> local
> >>>> version of the code, but since we are now working towards
> >>>> upstreaming all our code, this could no longer be ignored.
> >>>>
> >>>> I guess my proposal is not to treat labels differently from any
> >>> other
> >>>> terminator - if a target wants to "schedule" labels - let it do
> so.
> >>> It
> >>>> actually works just fine for Hexagon... so if we can change this
> >>>> assert to something like:
> >>>>
> >>>>
> >>>> assert((!MI->isTerminator() || CanHandleTerminators) && "Cannot
> >>>> schedule terminators or labels!");
> >>>>
> >>>> It would equal label in rights with any other terminator. What do
> >>> you
> >>>> think?
> >>>
> >>> Sure, as long as CanHandleTerminators is commented to that effect.
> >>>
> >>> -Andy
> >> <CanHandleTerminators.patch>
> >
> > _______________________________________________
> > llvm-commits mailing list
> > llvm-commits at cs.uiuc.edu
> > http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits

-------------- next part --------------
A non-text attachment was scrubbed...
Name: CanHandleTerminators_2.patch
Type: application/octet-stream
Size: 576 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20130212/09b6d423/attachment.obj>