Fix SelectionDAG::ReplaceAllUsesWith to behave correctly when

uses are added to the From node while it is processing From's
use list, because of automatic local CSE. The fix is to avoid
visiting any new uses.

Fix a few places in the DAGCombiner that assumed that after
a RAUW call, the From node has no users and may be deleted.

This fixes PR3018.

git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@62533 91177308-0d34-0410-b5e6-96231b3b80d8

diff --git a/lib/CodeGen/SelectionDAG/DAGCombiner.cpp b/lib/CodeGen/SelectionDAG/DAGCombiner.cpp
index 67a419aac9084c95f35779994061bef3b8291912..0d90fc0748798f85b90ddbace6f599c43865cfa5 100644 (file)
--- a/lib/CodeGen/SelectionDAG/DAGCombiner.cpp
+++ b/lib/CodeGen/SelectionDAG/DAGCombiner.cpp
@@ -522,12 +522,17 @@ SDValue DAGCombiner::CombineTo(SDNode *N, const SDValue *To, unsigned NumTo,
     }
   }
   
-  // Nodes can be reintroduced into the worklist.  Make sure we do not
-  // process a node that has been replaced.
-  removeFromWorkList(N);
+  // Finally, if the node is now dead, remove it from the graph.  The node
+  // may not be dead if the replacement process recursively simplified to
+  // something else needing this node.
+  if (N->use_empty()) {
+    // Nodes can be reintroduced into the worklist.  Make sure we do not
+    // process a node that has been replaced.
+    removeFromWorkList(N);
   
-  // Finally, since the node is now dead, remove it from the graph.
-  DAG.DeleteNode(N);
+    // Finally, since the node is now dead, remove it from the graph.
+    DAG.DeleteNode(N);
+  }
   return SDValue(N, 0);
 }
 
@@ -658,12 +663,17 @@ void DAGCombiner::Run(CombineLevel AtLevel) {
     for (unsigned i = 0, e = N->getNumOperands(); i != e; ++i)
       AddToWorkList(N->getOperand(i).getNode());
       
-    // Nodes can be reintroduced into the worklist.  Make sure we do not
-    // process a node that has been replaced.
-    removeFromWorkList(N);
+    // Finally, if the node is now dead, remove it from the graph.  The node
+    // may not be dead if the replacement process recursively simplified to
+    // something else needing this node.
+    if (N->use_empty()) {
+      // Nodes can be reintroduced into the worklist.  Make sure we do not
+      // process a node that has been replaced.
+      removeFromWorkList(N);
     
-    // Finally, since the node is now dead, remove it from the graph.
-    DAG.DeleteNode(N);
+      // Finally, since the node is now dead, remove it from the graph.
+      DAG.DeleteNode(N);
+    }
   }
   
   // If the root changed (e.g. it was a dead load, update the root).

diff --git a/lib/CodeGen/SelectionDAG/SelectionDAG.cpp b/lib/CodeGen/SelectionDAG/SelectionDAG.cpp
index b83d537e372231673d0e60e53cb34bf1f8733269..d8124e6d7c34216c992f7e1676a4fabf1667a6bb 100644 (file)
--- a/lib/CodeGen/SelectionDAG/SelectionDAG.cpp
+++ b/lib/CodeGen/SelectionDAG/SelectionDAG.cpp
@@ -4388,9 +4388,16 @@ void SelectionDAG::ReplaceAllUsesWith(SDValue FromN, SDValue To,
          "Cannot replace with this method!");
   assert(From != To.getNode() && "Cannot replace uses of with self");
 
-  while (!From->use_empty()) {
-    SDNode::use_iterator UI = From->use_begin();
+  // Iterate over all the existing uses of From. This specifically avoids
+  // visiting any new uses of From that arrise while the replacement is
+  // happening, because any such uses would be the result of CSE: If an
+  // existing node looks like From after one of its operands is replaced
+  // by To, we don't want to replace of all its users with To too.
+  // See PR3018 for more info.
+  SDNode::use_iterator UI = From->use_begin(), UE = From->use_end();
+  while (UI != UE) {
     SDNode *U = *UI;
+    do ++UI; while (UI != UE && *UI == U);
 
     // This node is about to morph, remove its old self from the CSE maps.
     RemoveNodeFromCSEMaps(U);
@@ -4437,9 +4444,12 @@ void SelectionDAG::ReplaceAllUsesWith(SDNode *From, SDNode *To,
   if (From == To)
     return;
 
-  while (!From->use_empty()) {
-    SDNode::use_iterator UI = From->use_begin();
+  // Iterate over just the existing users of From. See the comments in
+  // the ReplaceAllUsesWith above.
+  SDNode::use_iterator UI = From->use_begin(), UE = From->use_end();
+  while (UI != UE) {
     SDNode *U = *UI;
+    do ++UI; while (UI != UE && *UI == U);
 
     // This node is about to morph, remove its old self from the CSE maps.
     RemoveNodeFromCSEMaps(U);
@@ -4480,9 +4490,12 @@ void SelectionDAG::ReplaceAllUsesWith(SDNode *From,
   if (From->getNumValues() == 1)  // Handle the simple case efficiently.
     return ReplaceAllUsesWith(SDValue(From, 0), To[0], UpdateListener);
 
-  while (!From->use_empty()) {
-    SDNode::use_iterator UI = From->use_begin();
+  // Iterate over just the existing users of From. See the comments in
+  // the ReplaceAllUsesWith above.
+  SDNode::use_iterator UI = From->use_begin(), UE = From->use_end();
+  while (UI != UE) {
     SDNode *U = *UI;
+    do ++UI; while (UI != UE && *UI == U);
 
     // This node is about to morph, remove its old self from the CSE maps.
     RemoveNodeFromCSEMaps(U);

diff --git a/test/CodeGen/X86/pr3018.ll b/test/CodeGen/X86/pr3018.ll
new file mode 100644 (file)
index 0000000..7d335ee
--- /dev/null
+++ b/test/CodeGen/X86/pr3018.ll
@@ -0,0 +1,8 @@
+; RUN: llvm-as < %s | llc -march=x86 | grep {orl       \$1}
+
+define i32 @test(i32 %A) nounwind {
+  %B = or i32 %A, 1
+  %C = or i32 %B, 1
+  %D = and i32 %C, 7057
+  ret i32 %D
+}