[PATCH] D57541: [DAGCombiner] Eliminate dead stores to stack.

[Clement Courbet via Phabricator via llvm-commits]

courbet marked 2 inline comments as done.
courbet added a comment.

I finally had time to track this down.

The issue was that we were missing a hasOneUse() check on the chain (we were only checking it on the store itself), so there could be other nodes dependind on the store (typically through tokenfactors), e.g.

  store ----> TF ---> LIFETIME_END
               \-----> some_user

We were deleting the store...



================
Comment at: llvm/trunk/lib/CodeGen/SelectionDAG/DAGCombiner.cpp:15524
+        Chains.push_back(Chain.getOperand(0));
+      }
+      break;
----------------
niravd wrote:
> Ahha! We never add the lifetime to the Alias list like we should if we dont' skip past it. I suspect this is why we had to revert this patch.
I think you meant to put this comment on the lifetime skipping in `GatherAllAliases`. But you're right, thanks.



Repository:
  rL LLVM

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D57541/new/

https://reviews.llvm.org/D57541