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

[Nirav Dave via Phabricator via llvm-commits]

niravd accepted this revision.
niravd added a comment.
This revision is now accepted and ready to land.

Nice cleanup! LGTM modulo nits.

>From my reading of the PPC backend, the PowerPC version is that the generic version is marginally improving debugability and there is one path in PPC target where the OldChain is not set and needs to be checked. Since the early return is semantically equivalent, I see no reason not to just add the initial check and remove the PPC splice operation.



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



================
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);
----------------
Nit: use getChain() for these two.


https://reviews.llvm.org/D33649