[PATCH] D35294: [CGP] Allow cycles during Phi traversal in OptimizaMemoryInst

[Eli Friedman via Phabricator via llvm-commits]

efriedma added inline comments.


================
Comment at: lib/CodeGen/CodeGenPrepare.cpp:4285
+    if (!Visited.insert(V).second)
+      continue;
 
----------------
skatkov wrote:
> efriedma wrote:
> > When you're looking through a PHI node, equality can be deceptive; induction variables are the "same", but can have a different value.   (We got into trouble with BasicAA doing that recently.) I'd like to see a comment explaining why this particular version of the algorithm is correct.
> Hi Eli, thank you for the good question. My current understanding as is follows. We are traversing through all paths from address up by use->def chain and finally found that ptr is computed as arithmetic expression of the form BaseGv + Base + Scale * Index + Offset, where Offset and Scale are constants while BaseGV, Base and Index are exact the same instructions.
> 
> As soon as all these are the same and we traversed by all paths these instruction dominates our ptr and our memory instruction. Instruction cannot change its value on the paths its dominates and our found expression will produce the same value on the path from definition of these instruction till our load and actually further. So we can safely add the computation of ptr right before our load.
> 
> Please correct me if I'm wrong or missed anything.
That looks right.

Please add that explanation as a comment in the code.


https://reviews.llvm.org/D35294