[PATCH] D28193: NewGVN: Add UnknownExpression and create them for things we can't symbolize. Kill fragile machinery for handling null expressions.

[Daniel Berlin via Phabricator via llvm-commits]

dberlin marked 2 inline comments as done.
dberlin added a comment.

So, note that i'm wrong about a testcase. This is already test by pr31491.ll.

It's just fixing it in a less fragile way.



================
Comment at: lib/Transforms/Scalar/NewGVN.cpp:644-648
+const UnknownExpression *NewGVN::createUnknownExpression(Instruction *I) {
+  auto *E = new (ExpressionAllocator) UnknownExpression(I);
+  E->setOpcode(I->getOpcode());
+  return E;
+}
----------------
davide wrote:
> I think we may want to add doxygen documentation to every `create*` function, but probably that can be done separately in a single pass (if you agree, of course).
Sure.
What do you want it to say?

There are only a few that have any special behavior.  :)



================
Comment at: lib/Transforms/Scalar/NewGVN.cpp:1392-1397
+    // Unlike other terminators, invokes can return values. Just give them all
+    // unique return values.
+    if (isa<InvokeInst>(I)) {
+      auto *Symbolized = createUnknownExpression(I);
+      performCongruenceFinding(I, Symbolized);
+    }
----------------
davide wrote:
> There's another terminator which can return values, i.e. `catchswitch`. Maybe we should handle it specially here as well? http://llvm.org/docs/LangRef.html#catchswitch-instruction
I think i can just change the test to be about whether the terminator is a void type.
I'll check.



https://reviews.llvm.org/D28193