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

[Andrew Trick]

Please commit. Thanks.
-Andy

On Feb 12, 2013, at 8:12 AM, Sergei Larin <slarin at codeaurora.org> wrote:

> 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
> 
> <CanHandleTerminators_2.patch>