Fix some bugs in SimplifyNodeWithTwoResults where it would call deletenode to

delete a node even if it was not dead in some cases.  Instead, just add it to
the worklist.  Also, make sure to use the CombineTo methods, as it was doing
things that were unsafe: the top level combine loop could touch dangling memory.

This fixes CodeGen/Generic/2008-01-25-dag-combine-mul.ll



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

2 files changed, 62 insertions, 41 deletions

diff --git a/lib/CodeGen/SelectionDAG/DAGCombiner.cpp b/lib/CodeGen/SelectionDAG/DAGCombiner.cpp
index 6510a0f..ee7ef19 100644
--- a/lib/CodeGen/SelectionDAG/DAGCombiner.cpp
+++ b/lib/CodeGen/SelectionDAG/DAGCombiner.cpp
@@ -277,7 +277,8 @@ namespace {
                                bool NotExtCompare = false);
     SDOperand SimplifySetCC(MVT::ValueType VT, SDOperand N0, SDOperand N1,
                             ISD::CondCode Cond, bool foldBooleans = true);
-    bool SimplifyNodeWithTwoResults(SDNode *N, unsigned LoOp, unsigned HiOp);
+    SDOperand SimplifyNodeWithTwoResults(SDNode *N, unsigned LoOp, 
+                                         unsigned HiOp);
     SDOperand ConstantFoldBIT_CONVERTofBUILD_VECTOR(SDNode *, MVT::ValueType);
     SDOperand BuildSDIV(SDNode *N);
     SDOperand BuildUDIV(SDNode *N);
@@ -586,6 +587,7 @@ void DAGCombiner::Run(bool RunningAfterLegalize) {
       continue;
     
     ++NodesCombined;
+    
     // If we get back the same node we passed in, rather than a new node or
     // zero, we know that the node must have defined multiple values and
     // CombineTo was used.  Since CombineTo takes care of the worklist 
@@ -604,7 +606,8 @@ void DAGCombiner::Run(bool RunningAfterLegalize) {
     if (N->getNumValues() == RV.Val->getNumValues())
       DAG.ReplaceAllUsesWith(N, RV.Val, &NowDead);
     else {
-      assert(N->getValueType(0) == RV.getValueType() && "Type mismatch");
+      assert(N->getValueType(0) == RV.getValueType() &&
+             N->getNumValues() == 1 && "Type mismatch");
       SDOperand OpV = RV;
       DAG.ReplaceAllUsesWith(N, &OpV, &NowDead);
     }
@@ -1311,6 +1314,7 @@ SDOperand DAGCombiner::visitSREM(SDNode *N) {
   // X%C to the equivalent of X-X/C*C.
   if (N1C && !N1C->isNullValue()) {
     SDOperand Div = DAG.getNode(ISD::SDIV, VT, N0, N1);
+    AddToWorkList(Div.Val);
     SDOperand OptimizedDiv = combine(Div.Val);
     if (OptimizedDiv.Val && OptimizedDiv.Val != Div.Val) {
       SDOperand Mul = DAG.getNode(ISD::MUL, VT, OptimizedDiv, N1);
@@ -1421,18 +1425,16 @@ SDOperand DAGCombiner::visitMULHU(SDNode *N) {
 /// compute two values. LoOp and HiOp give the opcodes for the two computations
 /// that are being performed. Return true if a simplification was made.
 ///
-bool DAGCombiner::SimplifyNodeWithTwoResults(SDNode *N,
-                                             unsigned LoOp, unsigned HiOp) {
+SDOperand DAGCombiner::SimplifyNodeWithTwoResults(SDNode *N, unsigned LoOp, 
+                                                  unsigned HiOp) {
   // If the high half is not needed, just compute the low half.
   bool HiExists = N->hasAnyUseOfValue(1);
   if (!HiExists &&
       (!AfterLegalize ||
        TLI.isOperationLegal(LoOp, N->getValueType(0)))) {
-    DAG.ReplaceAllUsesOfValueWith(SDOperand(N, 0),
-                                  DAG.getNode(LoOp, N->getValueType(0),
-                                              N->op_begin(),
-                                              N->getNumOperands()));
-    return true;
+    SDOperand Res = DAG.getNode(LoOp, N->getValueType(0), N->op_begin(),
+                                N->getNumOperands());
+    return CombineTo(N, Res, Res);
   }
 
   // If the low half is not needed, just compute the high half.
@@ -1440,74 +1442,62 @@ bool DAGCombiner::SimplifyNodeWithTwoResults(SDNode *N,
   if (!LoExists &&
       (!AfterLegalize ||
        TLI.isOperationLegal(HiOp, N->getValueType(1)))) {
-    DAG.ReplaceAllUsesOfValueWith(SDOperand(N, 1),
-                                  DAG.getNode(HiOp, N->getValueType(1),
-                                              N->op_begin(),
-                                              N->getNumOperands()));
-    return true;
+    SDOperand Res = DAG.getNode(HiOp, N->getValueType(1), N->op_begin(),
+                                N->getNumOperands());
+    return CombineTo(N, Res, Res);
   }
 
   // If both halves are used, return as it is.
   if (LoExists && HiExists)
-    return false;
+    return SDOperand();
 
   // If the two computed results can be simplified separately, separate them.
-  bool RetVal = false;
   if (LoExists) {
     SDOperand Lo = DAG.getNode(LoOp, N->getValueType(0),
                                N->op_begin(), N->getNumOperands());
+    AddToWorkList(Lo.Val);
     SDOperand LoOpt = combine(Lo.Val);
-    if (LoOpt.Val && LoOpt != Lo &&
-        TLI.isOperationLegal(LoOpt.getOpcode(), LoOpt.getValueType())) {
-      RetVal = true;
-      DAG.ReplaceAllUsesOfValueWith(SDOperand(N, 0), LoOpt);
-    } else
-      DAG.DeleteNode(Lo.Val);
+    if (LoOpt.Val && LoOpt.Val != Lo.Val &&
+        TLI.isOperationLegal(LoOpt.getOpcode(), LoOpt.getValueType()))
+      return CombineTo(N, LoOpt, LoOpt);
   }
 
   if (HiExists) {
     SDOperand Hi = DAG.getNode(HiOp, N->getValueType(1),
                                N->op_begin(), N->getNumOperands());
+    AddToWorkList(Hi.Val);
     SDOperand HiOpt = combine(Hi.Val);
     if (HiOpt.Val && HiOpt != Hi &&
-        TLI.isOperationLegal(HiOpt.getOpcode(), HiOpt.getValueType())) {
-      RetVal = true;
-      DAG.ReplaceAllUsesOfValueWith(SDOperand(N, 1), HiOpt);
-    } else
-      DAG.DeleteNode(Hi.Val);
+        TLI.isOperationLegal(HiOpt.getOpcode(), HiOpt.getValueType()))
+      return CombineTo(N, HiOpt, HiOpt);
   }
-
-  return RetVal;
+  return SDOperand();
 }
 
 SDOperand DAGCombiner::visitSMUL_LOHI(SDNode *N) {
-  
-  if (SimplifyNodeWithTwoResults(N, ISD::MUL, ISD::MULHS))
-    return SDOperand();
+  SDOperand Res = SimplifyNodeWithTwoResults(N, ISD::MUL, ISD::MULHS);
+  if (Res.Val) return Res;
 
   return SDOperand();
 }
 
 SDOperand DAGCombiner::visitUMUL_LOHI(SDNode *N) {
-  
-  if (SimplifyNodeWithTwoResults(N, ISD::MUL, ISD::MULHU))
-    return SDOperand();
+  SDOperand Res = SimplifyNodeWithTwoResults(N, ISD::MUL, ISD::MULHU);
+  if (Res.Val) return Res;
 
   return SDOperand();
 }
 
 SDOperand DAGCombiner::visitSDIVREM(SDNode *N) {
-  
-  if (SimplifyNodeWithTwoResults(N, ISD::SDIV, ISD::SREM))
-    return SDOperand();
+  SDOperand Res = SimplifyNodeWithTwoResults(N, ISD::SDIV, ISD::SREM);
+  if (Res.Val) return Res;
   
   return SDOperand();
 }
 
 SDOperand DAGCombiner::visitUDIVREM(SDNode *N) {
-  
-  if (SimplifyNodeWithTwoResults(N, ISD::UDIV, ISD::UREM))
-    return SDOperand();
+  SDOperand Res = SimplifyNodeWithTwoResults(N, ISD::UDIV, ISD::UREM);
+  if (Res.Val) return Res;
   
   return SDOperand();
 }
diff --git a/test/CodeGen/Generic/2008-01-25-dag-combine-mul.ll b/test/CodeGen/Generic/2008-01-25-dag-combine-mul.ll
new file mode 100644
index 0000000..4b25444
--- /dev/null
+++ b/test/CodeGen/Generic/2008-01-25-dag-combine-mul.ll
@@ -0,0 +1,31 @@
+; RUN: llvm-as < %s | llc
+; rdar://5707064
+
+define i32 @f(i16* %pc) {
+entry:
+	%acc = alloca i64, align 8		; <i64*> [#uses=4]
+	%tmp97 = load i64* %acc, align 8		; <i64> [#uses=1]
+	%tmp98 = and i64 %tmp97, 4294967295		; <i64> [#uses=1]
+	%tmp99 = load i64* null, align 8		; <i64> [#uses=1]
+	%tmp100 = and i64 %tmp99, 4294967295		; <i64> [#uses=1]
+	%tmp101 = mul i64 %tmp98, %tmp100		; <i64> [#uses=1]
+	%tmp103 = lshr i64 %tmp101, 0		; <i64> [#uses=1]
+	%tmp104 = load i64* %acc, align 8		; <i64> [#uses=1]
+	%.cast105 = zext i32 32 to i64		; <i64> [#uses=1]
+	%tmp106 = lshr i64 %tmp104, %.cast105		; <i64> [#uses=1]
+	%tmp107 = load i64* null, align 8		; <i64> [#uses=1]
+	%tmp108 = and i64 %tmp107, 4294967295		; <i64> [#uses=1]
+	%tmp109 = mul i64 %tmp106, %tmp108		; <i64> [#uses=1]
+	%tmp112 = add i64 %tmp109, 0		; <i64> [#uses=1]
+	%tmp116 = add i64 %tmp112, 0		; <i64> [#uses=1]
+	%tmp117 = add i64 %tmp103, %tmp116		; <i64> [#uses=1]
+	%tmp118 = load i64* %acc, align 8		; <i64> [#uses=1]
+	%tmp120 = lshr i64 %tmp118, 0		; <i64> [#uses=1]
+	%tmp121 = load i64* null, align 8		; <i64> [#uses=1]
+	%tmp123 = lshr i64 %tmp121, 0		; <i64> [#uses=1]
+	%tmp124 = mul i64 %tmp120, %tmp123		; <i64> [#uses=1]
+	%tmp126 = shl i64 %tmp124, 0		; <i64> [#uses=1]
+	%tmp127 = add i64 %tmp117, %tmp126		; <i64> [#uses=1]
+	store i64 %tmp127, i64* %acc, align 8
+	ret i32 0
+}