[PATCH] D13358: InstCombine: Fold comparisons between unguessable allocas and other pointers

[Hans Wennborg via llvm-commits]

hans added a comment.

> Right now GetUnderlyingObject does not look through PHI nodes, but if it starts to at some point, this change will fold %cmp [...]


That would be correct, though. Or are you saying there's a problem there?


================
Comment at: lib/Transforms/InstCombine/InstCombineCompares.cpp:786
@@ +785,3 @@
+        return nullptr; // Found more than one cmp.
+      continue;
+    } else if (auto *Intrin = dyn_cast<IntrinsicInst>(V)) {
----------------
sanjoy wrote:
> How about `assert(V == &ICI)` here?
It might find a different cmp first though, and then bail out later when it finds ICI.

================
Comment at: lib/Transforms/InstCombine/InstCombineCompares.cpp:792
@@ +791,3 @@
+        case Intrinsic::dbg_declare: case Intrinsic::dbg_value:
+        case Intrinsic::memcpy: case Intrinsic::memmove: case Intrinsic::memset:
+          continue;
----------------
sanjoy wrote:
> hans wrote:
> > majnemer wrote:
> > > Can't `memcpy` escape the value just like a `StoreInst` would?
> > It's not possible to memcpy the value directly. The value would have to be stored in an object, and then that could be memcpy'd. But we'd notice the value escaping through the store.
> Might be helpful to note that `memset` is safe only because you don't allow `ptrtoint`.
Thanks, I'll put a comment here.


http://reviews.llvm.org/D13358