[PATCH] D43198: [DAG] Enforce stricter NodeId invariant during Instruction selection

[Nirav Dave via Phabricator via llvm-commits]

niravd marked an inline comment as done.
niravd added a comment.

> Looks overall reasonable. Is there any performance impact of EnforceNodeIdInvariant?

We have to make a flood fill pass of the furrow between selected nodes which is potentially large if we select nodes wildly out of order. But that requires some target-specific munging which doesn't seem likely; I have not observed a depth of more than 2.



================
Comment at: llvm/lib/CodeGen/SelectionDAG/SelectionDAGISel.cpp:985
+      if (UId > 0) {
+        U->setNodeId(-1 * UId);
+        Nodes.push_back(U);
----------------
jyknight wrote:
> Don't want to conflict with existing usage of -1, right? How about `(-UId) -1`?
Okay


================
Comment at: llvm/lib/CodeGen/SelectionDAG/SelectionDAGISel.cpp:3028
   case ISD::AssertZext:
-    CurDAG->ReplaceAllUsesOfValueWith(SDValue(NodeToMatch, 0),
-                                      NodeToMatch->getOperand(0));
+    ReplaceUses(SDValue(NodeToMatch, 0), NodeToMatch->getOperand(0));
     CurDAG->RemoveDeadNode(NodeToMatch);
----------------
jyknight wrote:
> Why not ReplaceNode?
We can't because there is no guarantee that NodeToMatch->getOperand(0) has only 1 value.


Repository:
  rL LLVM

https://reviews.llvm.org/D43198