[PATCH] D28934: Write a new SSAUpdater

Matthias Braun via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Fri Jan 20 17:49:13 PST 2017

MatzeB added a comment.

I am obviously a fan of the algorithm and biased :)

- Interesting tweak with the visited set (compared to the paper), guess that saves us RAUWs in many cases at the cost of extra visited set lookups.
- I am surprised to find readVariableBeforeDef() as public API, in my experience outside users of the algorithms do not need it (but maybe you have a good use case). I probably would have named it readVariableAtBlockBegin() or readVariableLiveIn() and the "normal" variant simply readVariable().

Comment at: lib/Transforms/Utils/SSAUpdaterNew.cpp:59
+    return CDI->second;
+  return readVariableAfterDef(Var, BB);
Shouldn't this be `readVariableBeforeDef(Var, BB)`?

Comment at: lib/Transforms/Utils/SSAUpdaterNew.cpp:85
+      // See if we see ourselves, due to a self-cycle
+      if (CurrentDef.lookup({Var, BB}) == PredVal)
+        continue;
Maybe `PredVal->getParent() == BB && isa<PHINode>(PredVal)` is slightly more efficient.


More information about the llvm-commits mailing list