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

[Chandler Carruth via llvm-commits]

chandlerc added inline comments.

================
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);
+  }
 
----------------
davidxl wrote:
> chandlerc wrote:
> > 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.
> One data point -- DSE is not the only client pass that triggers the problem. We have seen GVN triggered one as well.   (I have not examined this patch in detail).
With out a very clear description of why the *same* limiting technique will work with two different consumers, I think it is a big mistake to put it into the API.


http://reviews.llvm.org/D15537