[PATCH] D28137: [Devirtualization] MemDep returns non-local !invariant.group dependencies

[Piotr Padlewski via Phabricator via llvm-commits]

Prazek added inline comments.


================
Comment at: lib/Analysis/MemoryDependenceAnalysis.cpp:340
+  // Non-local invariant group dependency indicates there is non local Def,
+  // which is better than local clobber and everything else.
+  if (InvariantGroupDependency.isNonLocal())
----------------
dberlin wrote:
> Err, doesn't it simply indicate there *may* be a non-local def because it hit the top of the block?
> (that's what it indicates for everything else)
> How does it indicate there *must* be one?
> 
Because this is what getInvariantGroupPointerDependency is doing and what documentation says.

"
   Returns Unknown if it does not
  /// find anything, and Def if it can be assumed that 2 instructions load or
  /// store the same value and NonLocal which indicate that non-local Def was
  /// found, which can be retrieved by calling getNonLocalPointerDependency
  /// with the same queried instruction.
"


================
Comment at: lib/Analysis/MemoryDependenceAnalysis.cpp:379
+      return Other;
+    return Best;
+  };
----------------
dberlin wrote:
> This loop is N^2, because each of these dominates calls make take O(N) time.
> 
ok, I will leave FIXME


================
Comment at: lib/Analysis/MemoryDependenceAnalysis.cpp:429
+                            MemDepResult::getDef(ClosestDependency), nullptr));
+  return MemDepResult::getNonLocal();
 }
----------------
dberlin wrote:
> As long as you are willing to commit to doing this right, i'm okay with it.
yep, implementing it in MSSA sounds like a real fun!


https://reviews.llvm.org/D28137