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

[Sanjoy Das via llvm-commits]

sanjoy 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` in

  ph:
   %k = alloca i32
   br label %loop
  
  loop:
   %t = phi i32* [ %k, %ph ], [ gep(%k, 1), %loop ]
   %cmp = icmp eq %k, %arg
   br i1 %cmp, label %loop, label %leave

I don't know if that ^ is something you want, but I wanted to point it out.


================
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)) {
----------------
How about `assert(V == &ICI)` here?

================
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;
----------------
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`.


http://reviews.llvm.org/D13358