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

Nirav Dave via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Tue Feb 5 07:30:59 PST 2019


niravd added a comment.

In D57541#1384961 <https://reviews.llvm.org/D57541#1384961>, @courbet wrote:

> > Also, you'll need to deal with the additional output from indexed stores.
>
> I think I'm fine because of the `hasOneUse()` check. I'm not sure how I should test this though, do you have a suggestion for some test IR for an indexed store ?


I believe the CombineTo will fail from too few values on indexed stores, even if the value isn't used. Better to check isIndexed() or do the unfolding back to an ADD/SUB.

I suspect the only way this would trigger currently is if an intrinsic lowers into an indexed store in ARM or AArch64.



================
Comment at: lib/CodeGen/SelectionDAG/DAGCombiner.cpp:15386
+
+  // We walk up the chains to find stores.
+  SmallVector<SDValue, 8> Chains = {N->getOperand(0)};
----------------
I think we should merge this with FindBetterChains. At the least we should have load/store aliasing be able to bypass unrelated lifetime nodes and vice versa.


Repository:
  rL LLVM

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

https://reviews.llvm.org/D57541





More information about the llvm-commits mailing list