[PATCH] D51664: [IR] Lazily number instructions for local dominance queries

[Jakub Kuderski via Phabricator via llvm-commits]

kuhar added inline comments.


================
Comment at: llvm/include/llvm/IR/BasicBlock.h:487
 
+#ifdef NDEBUG
+/// In release builds, this is a no-op. For !NDEBUG builds, the checks are
----------------
Does it make sense to disable this when EXPENSIVE_CHECKS are set?


================
Comment at: llvm/lib/Analysis/OrderedInstructions.cpp:32
   if (InstA->getParent() == InstB->getParent())
     return localDominates(InstA, InstB);
   return DT->dominates(InstA->getParent(), InstB->getParent());
----------------
Is the separate function `localDominates` still needed? Seems like the body is trivial and could be inlined here?


================
Comment at: llvm/lib/IR/BasicBlock.cpp:498
+  for (Instruction &I : *this)
+    I.Order = Order++;
+
----------------
Is it possible to use noncontiguous  indices? If the indices are spread apart, you should be able to perform most insertions without renumbering instructions.


================
Comment at: llvm/lib/IR/SymbolTableListTraitsImpl.h:94
+  ItemParentClass *NewIP = getListOwner();
+  invalidateParentIListOrdering(NewIP);
+
----------------
Isn't it enough to invalidate only the indices of instructions that follow the first inserted one?


https://reviews.llvm.org/D51664