[PATCH] D30471: [DAG] Relax conditions under stores of loaded values can be merged

[Eli Friedman via Phabricator via llvm-commits]

efriedma added inline comments.


================
Comment at: lib/CodeGen/SelectionDAG/DAGCombiner.cpp:12015
+                  (isa<ConstantFPSDNode>(OtherST->getValue()) ||
+                   isa<LoadSDNode>(OtherST->getValue()))))
               continue;
----------------
niravd wrote:
> efriedma wrote:
> > Is this a correctness check, or a profitability check?  At first glance, it isn't obvious why we're checking any of this here.
> Folded in some pending code cleanup that makes this a bit mroe clear. This is a filter to make sure we only get mergeable candidates that would be profitable.
> 
Okay, that's better.


================
Comment at: lib/CodeGen/SelectionDAG/DAGCombiner.cpp:12430
+      // Reset Token0's input from itself to Ld's output chain.
+      CombineTo(Token0.getNode(), Token);
+      AddToWorklist(Ld);
----------------
Ah, I see what you're doing.  I really don't like the fact that you're creating a TokenFactor which uses itself, even temporarily... but I'm not sure what the best approach is.  Could you ask on llvmdev?


https://reviews.llvm.org/D30471