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

[Justin Holewinski]

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
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20130319/894e3b58/attachment.html>