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

[Sergei Larin]

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.

Sergei

---
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
-------------- next part --------------
A non-text attachment was scrubbed...
Name: CanHandleTerminators.patch
Type: application/octet-stream
Size: 556 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20130211/acb62a62/attachment.obj>