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

[Andrew Trick]

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.

Your patch looks fine but doesn't go far enough. I'd like to add an IROrder field to SDNode (eventually we might be able to make it redundant with NodeId, although there would be temporary points at which nodes have to share an IROrder). That would remove any concerns about the compile-time of potentially frequent DenseMap lookup. But I really want to do it to help ensure that IROrder remains present and valid as a topological order. Then we don't need a "source order" scheduler at all, which would be really great. Just emit MIs for the selected nodes in place, and break some physreg interferences.

Ideally, anyone who creates an SDNode needs to track down the IROrder. Rather than propagating IROrder when we replace all uses, we would do it when we morph or CSE the node, similar to DebugLoc. Ensuring topological order is another aspect of the problem that can be dealt with later.

It's a big infrastructure project. But if you find any part of this plan will help you, progress toward that goal is welcome.

-Andy

> 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>_______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20130320/a1f6964b/attachment.html>