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

Nirav Dave via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Mon Jun 12 06:36:07 PDT 2017

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?

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. 


More information about the llvm-commits mailing list