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

[Justin Holewinski]

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
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20130319/9a46f0ea/attachment.html>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-Propagate-DAG-node-ordering-during-type-legalization.patch
Type: application/octet-stream
Size: 12104 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20130319/9a46f0ea/attachment.obj>