[PATCH] D33649: [DAG] add helper to bind memop chains; NFCI

[Nirav Dave via Phabricator via llvm-commits]

niravd added inline comments.


================
Comment at: include/llvm/CodeGen/SelectionDAG.h:1179
+  /// the same relative memory dependency position as the old load.
+  void bindChains(LoadSDNode *Old, SDValue New);
+
----------------
spatel wrote:
> niravd wrote:
> > It'd be nice if the name bindChain could probably be clearer. Maybe something along the lines of  "markSymettricMemoryOrderings".
> > 
> Does "makeEquivalentMemoryOrdering" capture the intent better?
SGTM.


================
Comment at: lib/CodeGen/SelectionDAG/SelectionDAG.cpp:7243
+  // uses of the old load's output chain to use that TokenFactor.
+  SDValue OldChain = SDValue(OldLoad, 1);
+  SDValue NewChain = SDValue(NewLoadVal.getNode(), 1);
----------------
spatel wrote:
> niravd wrote:
> > Nit: use getChain() for these two.
> IIUC, getChain() gives us the chain operand rather than the chain result. When I try this, we get many test failures because, for example, the chain operand may not have exactly the 2 operands that UpdateNodeOperands() is expecting for a LoadSDNode.
Right, of course. Nevermind. 


https://reviews.llvm.org/D33649