[PATCH] D39604: [DAGCombine] Move AND nodes to multiple load leaves

[Nirav Dave via Phabricator via llvm-commits]

niravd added inline comments.


================
Comment at: lib/CodeGen/SelectionDAG/DAGCombiner.cpp:3755
+      // Allow one node which will masked along with any loads found.
+      if (NodeToMask)
+        return false;
----------------
If  you pull out the the checking and recording of NodeToMask to after the switch and return immediately on sucessful cases, you can out the default case and allows us to succeed with an "and of load" if one of the loads is not a valid extLoad. 


================
Comment at: lib/CodeGen/SelectionDAG/DAGCombiner.cpp:3805
+                            SDValue(N, 0), Mask);
+  SDValue N0 = SDValue(N, 0);
+  for (auto *User : N->uses()) {
----------------
You should be able to replace most of this by using ReplaceAllUsesWith and then using UpdateNodeOperands to fix up the operands of the And Node. i.e.:

DAG.ReplaceAllUsesWith(N, And);
DAG.UpdateNodeOperands(And, SDValue(N,0), Mask));

It's probably worth inlining this function as well. 



================
Comment at: lib/CodeGen/SelectionDAG/DAGCombiner.cpp:3834
+  // No need to do anything if the and directly uses a load.
+  if (isa<LoadSDNode>(N->getOperand(0)))
+    return false;
----------------
IMO, this fast fail check isn't worth while as it's shaving off a smallish constant amount of wotk. 


https://reviews.llvm.org/D39604