[PATCH] D44282: [PR16756] JumpThreading: explicitly update SSA rather than use SSAUpdater.

[Mikhail Zolotukhin via llvm-commits]

> On Mar 28, 2018, at 4:38 PM, Daniel Berlin <dberlin at dberlin.org> wrote:
> 
> I'm ... confused.
> 
> 
> On Wed, Mar 28, 2018 at 4:33 PM, Michael Zolotukhin via Phabricator <reviews at reviews.llvm.org <mailto:reviews at reviews.llvm.org>> wrote:
> mzolotukhin added a comment.
> 
> > I don't mind this approach, but as discussed offline we should consider also moving LCSSA to make sure the API makes sense.
> >  In general, what's your plan for this? You want it to replace the SSA updater for all the instances in llvm? If so, we should carefully plan the transition costs.
> 
> I looked into LCSSA, and indeed it seems that it also can be improved. I tried direct replacement of the old SSAUpdater with the new one, but that didn't give any benefits.
> 
> Before you said "(although the new implementation still was significantly faster than the existing one)”
Let’s clarify this. “Although the new implementation still was significantly faster” was about another approach based on D28934.

This patch (let’s call it SSAUpdaterBulk) is about another approach, which is closer to what we have in the trunk (let’s call it SSAUpdater) except that we 1) don’t compute DT but instead use DT provided by the user, 2) can do bulk updates that share the same DT and IDF when possible. If the use case is that we have a lot of uses to rewrite and they share the same definitions, SSAUpdaterBulk should be faster than SSAUpdater, as it won’t compute DT at all and will compute IDF only once for all these defs and uses. That’s what happens in JumpThreading and that’s why we see the gains there. If, however, we have an opposite case, where we rewrite one use at a time, we recompute IDF every time we do the rewrite, which is not so much faster and sometimes probably even slower than what we have in SSAUpdater. That’s probably what happened when I tried a direct replacement of SSAUpdater with SSAUpdaterBulk in LCSSA. What I was saying was that it should be possible to rewrite LCSSA in a way allowing to get benefits of SSAUpdaterBulk.

Hopefully it helps a it and sorry if it’s been confusing!

Thanks,
Michael
> 
> 

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