[PATCH] D28934: Write a new SSAUpdater

Davide Italiano via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Fri Jan 20 10:36:09 PST 2017

davide added a comment.

In https://reviews.llvm.org/D28934#651355, @dberlin wrote:

> In https://reviews.llvm.org/D28934#651323, @sanjoy wrote:
> > I've never really heard people complain about our current `SSAUpdater` (about it being too slow or difficult to use) -- well except in this review :).  Is there a specific problem you're trying to address with this?
> FWIW: It's been a source of sorrow before.
>  People kind of randomly try to figure out what order to call things in, because it's not clear what the terminology it uses means, nor is it clear what the right sequence necessarily is to get the right result.
> As for speed, we wrote an entire separate linear time phi placement pass (see promotememtoreg, and more particularly, idfcalculator) just because the current SSAUpdater was too slow and not easily understandable.
>  It's also used for ADCE and MemorySSA.
> This particular patch came about because i needed an SSAUpdater workalike for MemorySSA updating, and i figured, what the heck, might as well see if anyone wants me to replace the existing one while i'm at it.
>  I started with the above, and then modified it for what i needed.
> If you stare at this vs the paper vs the existing SSAUpdater, you can clearly see the paper (and this impl) is a slightly more refined version of the renaming algorithm, with a completely different phi insertion scheme shoved on top.
>  (i'm not sure if they both share some common lineage, because otherwise, that's ... yeah)
> Our current SSAUpdater tries to iteratively compute dominators, dominance frontiers, and then phi placement for those. It's actually N^3 in the number of variables, and in some cases, badly so.

While I don't think speed is the only factor we should consider, I don't think I ever found a case where compile time for SSAUpdater skyrockets, but this doesn't mean there aren't any. Did you find a case where this actually matters, out of curiosity?


More information about the llvm-commits mailing list