[PATCH] D23184: Rewrite domination verifier to handle local domination as well.

[George Burgess IV via llvm-commits]

george.burgess.iv accepted this revision.
george.burgess.iv added a comment.
This revision is now accepted and ready to land.

LGTM with comments, thanks.


================
Comment at: include/llvm/Transforms/Utils/MemorySSA.h:594
@@ +593,3 @@
+  /// dominates Use \p B.
+  bool dominates(const MemoryAccess *A, const Use &B) const;
+
----------------
Do we see this being used outside of verification? If not, make this private, please.

================
Comment at: lib/Transforms/Utils/MemorySSA.cpp:1872
@@ -1891,1 +1871,3 @@
+      for (const Use &U : MP->uses())
+        assert(dominates(MP, U) && "Memory PHI does not dominate it's uses");
     for (Instruction &I : B) {
----------------
Looks like this definition is provided if `NDEBUG` is defined. Please braces around the assert. :)

================
Comment at: lib/Transforms/Utils/MemorySSA.cpp:1879
@@ -1913,1 +1878,3 @@
+      for (const Use &U : MD->uses())
+        assert(dominates(MD, U) && "Memory Def does not dominate it's uses");
     }
----------------
Same as above


https://reviews.llvm.org/D23184