[PATCH] D15537: limit the number of instructions per block examined by dead store elimination

[Chandler Carruth via llvm-commits]

chandlerc added a subscriber: chandlerc.
chandlerc added a comment.

FWIW, I think it is reasonable to workaround quadratic compile time problems with temporary limits until a well scaling algorithm lands. However, I don't think this patch as-is is the correct approach.


================
Comment at: lib/Analysis/MemoryDependenceAnalysis.cpp:455-461
@@ -449,3 +454,9 @@
     const MemoryLocation &MemLoc, bool isLoad, BasicBlock::iterator ScanIt,
-    BasicBlock *BB, Instruction *QueryInst) {
+    BasicBlock *BB, Instruction *QueryInst, unsigned *Limit) {
+
+  if (!Limit) {
+    unsigned DefaultLimit = BlockScanLimit;
+    return getSimplePointerDependencyFrom(MemLoc, isLoad, ScanIt, BB, QueryInst,
+                                          &DefaultLimit);
+  }
 
----------------
Exposing the search limit of MemDep and allowing the dynamic search depth to be surfaced in the API seems like a really bad API.

Fundamentally, I think we should first bound the DSE scan when it has to call through to MemDep, which this effectively does, but in a very round-about way.

Then, if it necessary to give MemDep increasingly small scan limits, you should just add a vanilla input here, and decrease that limit in the DSE loop below rather than carrying-over the limit from one query to another query. But I'd really like to understand if all you actually need is to limit the DSE scan.


http://reviews.llvm.org/D15537