[PATCH] D28181: [MemDep] NFC walk invariant.group graph only down

[Piotr Padlewski via Phabricator via llvm-commits]

Prazek marked an inline comment as done.
Prazek added a comment.

BTW I think I also don't need SeenOperands set now because there is no way I could insert the same operand twice



================
Comment at: lib/Analysis/MemoryDependenceAnalysis.cpp:367
     assert(Ptr);
-    if (isa<GlobalValue>(Ptr))
-      continue;
-
-    // Value comes from bitcast: Ptr = bitcast x. Insert x.
-    if (auto *BCI = dyn_cast<BitCastInst>(Ptr))
-      TryInsertToQueue(BCI->getOperand(0));
-    // Gep with zeros is equivalent to bitcast.
-    // FIXME: we are not sure if some bitcast should be canonicalized to gep 0
-    // or gep 0 to bitcast because of SROA, so there are 2 forms. When typeless
-    // pointers will be upstream then both cases will be gone (and this BFS
-    // also won't be needed).
-    if (auto *GEP = dyn_cast<GetElementPtrInst>(Ptr))
-      if (GEP->hasAllZeroIndices())
-        TryInsertToQueue(GEP->getOperand(0));
+    assert(!isa<GlobalValue>(Ptr));
 
----------------
reames wrote:
> Don't you need to check this before the loop?  (i.e. why did this become an assert rather than a test?)
Good catch. There is check for GlobalValue on beginning of the function, but it check for pointerOperand.
So if I will check pointer after stripping, then this will be valid. I will write test for that.


================
Comment at: lib/Analysis/MemoryDependenceAnalysis.cpp:381
+      // Gep with zeros is equivalent to bitcast.
+      // FIXME: we are not sure if some bitcast should be canonicalized to gep 0
+      // or gep 0 to bitcast because of SROA, so there are 2 forms. When typeless
----------------
reames wrote:
> Wasn't this comment in the other change you landed?
I just moved it from line 372


https://reviews.llvm.org/D28181