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

[Max Kazantsev via Phabricator via llvm-commits]

mkazantsev added inline comments.


================
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.
----------------
reames wrote:
> As noted, calling this here makes this operation unexpected O(Inst in F) in the worst case.  
> 
> Please check only the containing block.  
This is intentional. If we have modified a block and haven't dropped cached info, we want to detect it as early as possible, specifically when we make *any* query to this tracking. Imagine we've screwed up invalidating a block `A`, then did 100 correct modifications and queries to other blocks and then requested something for `A`. The assertion will fail, and it will be very hard to figure out what and when has happened.

On the other hand, in current strategy we will be able to detect that we've screwed up whenever any query is made, which was the entire point of making this patch.

I don't think that win on compile time in debug mode is worth making investigation of functional bugs more complex.


https://reviews.llvm.org/D51523