[PATCH] D22527: Make MemorySSA::dominates/locallydominates constant time

[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 a few comments; thanks for the patch!


================
Comment at: include/llvm/Transforms/Utils/MemorySSA.h:635
@@ +634,3 @@
+  // global.
+  mutable DenseMap<const BasicBlock *, bool> BlockNumberingValid;
+  mutable DenseMap<const MemoryAccess *, unsigned long> BlockNumbering;
----------------
Given that we only ever store `true` for the value, can this be a `DenseSet` instead?

================
Comment at: lib/Transforms/Utils/MemorySSA.cpp:1205
@@ -1207,3 +1204,3 @@
 
 MemoryPhi *MemorySSA::createMemoryPhi(BasicBlock *BB) {
   assert(!getMemoryAccess(BB) && "MemoryPhi already exists for this BB");
----------------
Do we need to invalidate numbering here, as well? (Alternatively, we could start all counts from 2, and always give a newly-created phi a value of 1, but that could also be done later as a part of the "number with a stride of N" optimization)

================
Comment at: lib/Transforms/Utils/MemorySSA.cpp:1582
@@ +1581,3 @@
+  assert(AL != nullptr && "Asking to renumber an empty block");
+  for (const auto &I : *AL) {
+    BlockNumbering[&I] = ++CurrentNumber;
----------------
Nit: Please remove braces


https://reviews.llvm.org/D22527