[PATCH] Propagate DAG node ordering during legalization and instruction selection

[Justin Holewinski]

On Tue, Mar 19, 2013 at 5:12 PM, Evan Cheng <evan.cheng at apple.com> wrote:

> I think the patch is fine. I would feel completely confident if you don't
> see these compile time swings though. Are you making sure your machine is
> as quiet as possible and you have disabled power management etc.?
>

The tests where I see occasional variation are very short compile times.
 My guess is some background process is waking up, and that's enough to
perturb the results.  I committed this in r177465.  I'll keep an eye on the
bots.


>
> I do have a comment about relying on source order though. If you are
> relying on source-order scheduling, you have probably already lost the
> battle. What you really want to do is to skip DAG instruction selection as
> much as possible and rely on machine scheduler.
>

I agree completely!  In the long term I want to move to a real scheduler,
but that's not an option in the short term due to other constraints.
 Hopefully we'll be able to do something better sometime soon.

Thanks for the help Evan!


>
> Evan
>
> On Mar 19, 2013, at 1:17 PM, Justin Holewinski <
> justin.holewinski at gmail.com> wrote:
>
> Updated patch attached.
>
> I've addressed the CSE during legalization issue.  Now ordering is only
> propagated if the new node does not have an ordering (is zero), or has an
> ordering that is greater than the replaced node.
>
> As for compile time, I think I may have had some other machine
> interference in the 8% figure.  I can't reproduce that now, and both LLVM
> unit tests and LNT are not showing any statistically significant
> differences.  I see some variation across runs in LNT, but it looks to be
> machine noise as I see both regressions and improvements in different
> benchmarks in different runs.  I see +/- 0.5% in the unit tests, but that
> goes both ways.  Both tests use release+asserts build.
>
>
> On Tue, Mar 19, 2013 at 1:07 PM, Justin Holewinski <
> justin.holewinski at gmail.com> wrote:
>
>> On Tue, Mar 19, 2013 at 12:47 PM, Justin Holewinski <
>> justin.holewinski at gmail.com> wrote:
>>
>>>
>>> On Tue, Mar 19, 2013 at 2:26 AM, Evan Cheng <evan.cheng at apple.com>wrote:
>>>
>>>>
>>>>
>>>> Sent from my iPad
>>>>
>>>> On Mar 18, 2013, at 2:02 PM, Justin Holewinski <
>>>> justin.holewinski at gmail.com> wrote:
>>>>
>>>> Compile-time impact is negligible for a release build on the unit
>>>> tests.  There is about an 8% impact with assertions enabled.
>>>>
>>>>
>>>> Unit tests are much too small for measuring compile time. 8% for
>>>> assertion build is massive. Why are there such large discrepancy?
>>>>
>>>
>>> I've been trying to get measurements from LNT, but I'm getting too much
>>> run-to-run variation.  A few benchmarks show significant changes (both
>>> positive and negative), but the affected benchmarks are different from run
>>> to run.  The vast majority of benchmarks show no change.  Do you have any
>>> suggestions for getting concrete compile time data?
>>>
>>
>> I should also note that the 8% was only on the LLVM tests, where isel is
>> a large chunk of the testing time.  Including the clang tests, the
>> difference is around 0.5% (with run-to-run variation).
>>
>>
>>>
>>>
>>>>
>>>> Evan
>>>>
>>>>
>>>> I'm not sure how CSE factors in here.  This propagation happens during
>>>> legalization.  I agree that it should be addressed, but I see it as a
>>>> separate issue.  Unless I'm missing some instance of CSE being performed
>>>> during legalization?
>>>>
>>>>
>>>> On Mon, Mar 18, 2013 at 4:04 PM, Jakob Stoklund Olesen <stoklund at 2pi.dk
>>>> > wrote:
>>>>
>>>>>
>>>>> On Mar 15, 2013, at 10:51 AM, Justin Holewinski <
>>>>> justin.holewinski at gmail.com> wrote:
>>>>>
>>>>> Thanks Jakob!
>>>>>
>>>>> Modified patch attached.  Anyone adverse to this change?
>>>>>
>>>>>
>>>>> +
>>>>> +  // Propagate ordering
>>>>> +  unsigned Order = DAG.GetOrdering(Op.getNode());
>>>>> +  DAG.AssignOrdering(Lo.getNode(), Order);
>>>>> +  DAG.AssignOrdering(Hi.getNode(), Order);
>>>>>
>>>>> Did you consider what happens when CSE kicks in? The new nodes may not
>>>>> be new, they could have been CSE'ed with an existing expression.
>>>>>
>>>>> When that happens, the ordering should be the earlier of the two.
>>>>>
>>>>> I think it would make sense to teach the SDNodeOrdering class about
>>>>> this.
>>>>>
>>>>> /jakob
>>>>>
>>>>>
>>>>
>>>>
>>>> --
>>>>
>>>> Thanks,
>>>>
>>>> Justin Holewinski
>>>>
>>>> _______________________________________________
>>>>
>>>> llvm-commits mailing list
>>>> llvm-commits at cs.uiuc.edu
>>>> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>>>>
>>>>
>>>
>>>
>>> --
>>>
>>> Thanks,
>>>
>>> Justin Holewinski
>>>
>>
>>
>>
>> --
>>
>> Thanks,
>>
>> Justin Holewinski
>>
>
>
>
> --
>
> Thanks,
>
> Justin Holewinski
>  <0001-Propagate-DAG-node-ordering-during-type-legalization.patch>
>
>
>


-- 

Thanks,

Justin Holewinski
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20130319/8e747ce7/attachment.html>