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

Justin Holewinski justin.holewinski at gmail.com
Mon Mar 18 14:02:39 PDT 2013


Compile-time impact is negligible for a release build on the unit tests.
 There is about an 8% impact with assertions enabled.

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


More information about the llvm-commits mailing list