[PATCH] D28168: [LV] Update dominator tree before fixing external IV users

Michael Kuperstein via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Tue Jan 3 13:59:16 PST 2017


mkuper added a comment.

In https://reviews.llvm.org/D28168#634469, @mssimpso wrote:

> In https://reviews.llvm.org/D28168#634459, @mkuper wrote:
>
> > My gut feeling is that we really want to update DT as we go.
> >  Even if all current uses of transform() except this one are safe (and I'm not at all sure about that), leaving this as is sounds very brittle, since any additional use of transform() can potentially break it.
>
>
> I totally agree. I'm happy to continue looking at this, but when I updated the DT at the beginning (right after we create the new loop structure), subsequent calls to transform caused the Expander to generate "new" canonical IVs in addition to the ones we already create. Any idea why it would do this? I haven't yet figured out why the Expander would be generating worse code after such a change.
>
> For the Expander, it wants an IV that starts at zero and steps by one. But LV generates IVs that step by VFxUF.


I'm not really familiar with the expander, unfortunately.
In any case, I'm ok with this patch going in in the meanwhile. I think it still moves us in the right direction of updating DT earlier. :-)


https://reviews.llvm.org/D28168





More information about the llvm-commits mailing list