[PATCH] D28934: Write a new SSAUpdater

Daniel Berlin via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Fri Jan 20 18:04:25 PST 2017

dberlin added a comment.

As for what's public:
The paper comments that for incremental updates, you want readVariableBeforeDef. This is equivalence to getValueAtMiddleOfBlock, which we make public in SSAUpdater.

The paper's API is actually identical to SSAUpdater's old API circa 2010, with the functions renamed.
The only difference is the more advanced phi placement algorithms.
I think Bob even has most of the marker algorithm right at one point if you go back through the file's history.

I think if we keep going down this route i would change the API.
Users shouldn't generally have to care where in the block they are.
The real problem on this front is loops.

goto loop

use (a)
def (a)
goto loop or somewhere else

For loop, you need

1. To call writeVariable of both defs so it's visible to the algorithm, otherwise, it will not get the value over the backedge.
2. But ask about the version before the def, because that's where the use is.

I think this should all be hidden, and orderedbb or something used to locate the relative position of the use to the defs.

(this is what i'm doing for MemorySSA, but memory ssa is easier because we have a single variable and every may-def creates a new version. Thus, i don't even need the user to tell me the defs, i can just find the closest def, etc)

Comment at: lib/Transforms/Utils/SSAUpdaterNew.cpp:59
+    return CDI->second;
+  return readVariableAfterDef(Var, BB);
MatzeB wrote:
> Shouldn't this be `readVariableBeforeDef(Var, BB)`?
I think you are commenting on an old version of the patch where it had that bug :)


More information about the llvm-commits mailing list