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

[Xinliang David Li via llvm-commits]

On Tue, Aug 23, 2016 at 12:04 PM, Philip Reames
<listmail at philipreames.com> wrote:
> reames added inline comments.
>
> ================
> Comment at: lib/Transforms/Scalar/DeadStoreElimination.cpp:490
> @@ -489,3 +489,3 @@
>    // Do a top-down walk on the BB.
>    for (BasicBlock::iterator BBI = BB.begin(), BBE = BB.end(); BBI != BBE; ) {
>      Instruction *Inst = &*BBI++;
> ----------------
> I'm wondering whether we can solve the practical problem with a caching change.
>
> What if we simply chose to cache the intermediate MDA results?  (Note that MDA does not do this for the getPointerDependencyFrom interface.)
>
> I'm picturing a simple cache structure of Map<pair<MemoryLoc, Instruction>, MemDepResult>
>
> This wouldn't reduce the worst case complexity (it could be each instruction has a different memory location associated with it which is may alias with all others), but how many unique memory locations do we see in practice?  And out of those, how many are mayalias (i.e. not a possible read or def which terminates a chain)?
>
>
>
> ================
> Comment at: lib/Transforms/Scalar/DeadStoreElimination.cpp:522
> @@ -521,3 +521,3 @@
>
>        if (LoadInst *DepLoad = dyn_cast<LoadInst>(SI->getValueOperand())) {
>          if (SI->getPointerOperand() == DepLoad->getPointerOperand() &&
> ----------------
> FYI: this patch clearly needs rebased


https://reviews.llvm.org/F2313386 is the rebased one.

While implementing caching is possible, I think we should avoid
reinventing the wheel in memorySSA, but to keep the fix as simple as
possible.  The new limit can be adjusted higher as its meaning has
changed.

David

>
> https://reviews.llvm.org/D15537
>
>
>