[PATCH] D33649: [DAG] add helper to bind memop chains; NFCI
Sanjay Patel via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Fri Jun 2 06:48:15 PDT 2017
spatel 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);
+
----------------
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);
----------------
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.
https://reviews.llvm.org/D33649
More information about the llvm-commits
mailing list