[PATCH] D51523: Return "[NFC] Add severe validation of InstructionPrecedenceTracking"

[Philip Reames via Phabricator via llvm-commits]

reames requested changes to this revision.
reames added inline comments.
This revision now requires changes to proceed.


================
Comment at: include/llvm/Analysis/InstructionPrecedenceTracking.h:51
   InstructionPrecedenceTracking(DominatorTree *DT)
       : OI(OrderedInstructions(DT)) {}
 
----------------
I really really think there should be a destructor here which checks to make sure the information is still valid.  If the info is no longer used, a client can call clear explicitly.


================
Comment at: lib/Analysis/InstructionPrecedenceTracking.cpp:28
     const BasicBlock *BB) {
+#ifndef NDEBUG
+  // Make sure that we are making this request to tracking which is up-to-date.
----------------
As noted, calling this here makes this operation unexpected O(Inst in F) in the worst case.  

Please check only the containing block.  


================
Comment at: lib/Analysis/InstructionPrecedenceTracking.cpp:67
+  unsigned NumNoSpecialBlocks = 0;
+  // Check that for every known block we have something cached for it.
+  for (auto *BB : KnownBlocks) {
----------------
This comment caused me to misread.  Something more along the lines of:
// For each block we have cached, make sure we cached the right answer


https://reviews.llvm.org/D51523