[PATCH] D29845: [SelectionDAG] Remove redundant stores more aggressively.

Sanjay Patel via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Fri Feb 17 07:48:19 PST 2017


spatel added reviewers: niravd, jyknight, andreadb, hfinkel.
spatel added a comment.

I don't know enough about TokenFactor or chains to review, so adding more reviewers still. :)
Note that the problem isn't limited to ARM: the first test manifests (and gets fixed with this patch) on x86. But not the 2nd test. I didn't dig in to see why. On PPC, early-cse catches the first test, so the problem isn't ever visible in the DAG (why don't the other targets run that too?). Like x86, the 2nd test isn't fixed on PPC.



================
Comment at: lib/CodeGen/SelectionDAG/SelectionDAG.cpp:7161
+    // no side-effects between a TokenFactor and its operands.
+    if (llvm::is_contained((*this)->ops(), Dest))
+      return true;
----------------
Don't need "llvm::" here?


================
Comment at: lib/CodeGen/SelectionDAG/SelectionDAG.cpp:7165
+    // reaches Dest.
     for (unsigned i = 0, e = getNumOperands(); i != e; ++i)
       if (!getOperand(i).reachesChainWithoutSideEffects(Dest, Depth-1))
----------------
It would be nice to convert this loop to (*this)->ops() as well for symmetry.


Repository:
  rL LLVM

https://reviews.llvm.org/D29845





More information about the llvm-commits mailing list