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

[Bill Wendling]

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