[SROA] Fold a PHI node if all its incoming values are the same

Summary:
Fixes PR20425.

During slice building, if all of the incoming values of a PHI node are the same, replace the PHI node with the common value. This simplification makes alloca's used by PHI nodes easier to promote.

Test Plan: Added three more tests in phi-and-select.ll

Reviewers: nlewycky, eliben, meheff, chandlerc

Reviewed By: chandlerc

Subscribers: zinovy.nis, hfinkel, baldrick, llvm-commits

Differential Revision: http://reviews.llvm.org/D4659

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

lib/Transforms/Scalar/SROA.cpp

@@ -324,6 +324,15 @@ static Value *foldSelectInst(SelectInst &SI) {
   return nullptr;
 }
 
+/// \brief A helper that folds a PHI node or a select.
+static Value *foldPHINodeOrSelectInst(Instruction &I) {
+  if (PHINode *PN = dyn_cast<PHINode>(&I)) {
+    // If PN merges together the same value, return that value.
+    return PN->hasConstantValue();
+  }
+  return foldSelectInst(cast<SelectInst>(I));
+}
+
 /// \brief Builder for the alloca slices.
 ///
 /// This class builds a set of alloca slices by recursively visiting the uses
@@ -646,57 +655,40 @@ private:
     return nullptr;
   }
 
-  void visitPHINode(PHINode &PN) {
+  void visitPHINodeOrSelectInst(Instruction &I) {
-    if (PN.use_empty())
+    assert(isa<PHINode>(I) || isa<SelectInst>(I));
-      return markAsDead(PN);
+    if (I.use_empty())
-    if (!IsOffsetKnown)
+      return markAsDead(I);
-      return PI.setAborted(&PN);
 
-    // See if we already have computed info on this node.
+    // TODO: We could use SimplifyInstruction here to fold PHINodes and
-    uint64_t &PHISize = PHIOrSelectSizes[&PN];
+    // SelectInsts. However, doing so requires to change the current
-    if (!PHISize) {
+    // dead-operand-tracking mechanism. For instance, suppose neither loading
-      // This is a new PHI node, check for an unsafe use of the PHI node.
+    // from %U nor %other traps. Then "load (select undef, %U, %other)" does not
-      if (Instruction *UnsafeI = hasUnsafePHIOrSelectUse(&PN, PHISize))
+    // trap either.  However, if we simply replace %U with undef using the
-        return PI.setAborted(UnsafeI);
+    // current dead-operand-tracking mechanism, "load (select undef, undef,
-    }
+    // %other)" may trap because the select may return the first operand
-
+    // "undef".
-    // For PHI and select operands outside the alloca, we can't nuke the entire
+    if (Value *Result = foldPHINodeOrSelectInst(I)) {
-    // phi or select -- the other side might still be relevant, so we special
-    // case them here and use a separate structure to track the operands
-    // themselves which should be replaced with undef.
-    // FIXME: This should instead be escaped in the event we're instrumenting
-    // for address sanitization.
-    if (Offset.uge(AllocSize)) {
-      S.DeadOperands.push_back(U);
-      return;
-    }
-
-    insertUse(PN, Offset, PHISize);
-  }
-
-  void visitSelectInst(SelectInst &SI) {
-    if (SI.use_empty())
-      return markAsDead(SI);
-    if (Value *Result = foldSelectInst(SI)) {
       if (Result == *U)
         // If the result of the constant fold will be the pointer, recurse
-        // through the select as if we had RAUW'ed it.
+        // through the PHI/select as if we had RAUW'ed it.
-        enqueueUsers(SI);
+        enqueueUsers(I);
       else
-        // Otherwise the operand to the select is dead, and we can replace it
+        // Otherwise the operand to the PHI/select is dead, and we can replace
-        // with undef.
+        // it with undef.
         S.DeadOperands.push_back(U);
 
       return;
     }
+
     if (!IsOffsetKnown)
-      return PI.setAborted(&SI);
+      return PI.setAborted(&I);
 
     // See if we already have computed info on this node.
-    uint64_t &SelectSize = PHIOrSelectSizes[&SI];
+    uint64_t &Size = PHIOrSelectSizes[&I];
-    if (!SelectSize) {
+    if (!Size) {
-      // This is a new Select, check for an unsafe use of it.
+      // This is a new PHI/Select, check for an unsafe use of it.
-      if (Instruction *UnsafeI = hasUnsafePHIOrSelectUse(&SI, SelectSize))
+      if (Instruction *UnsafeI = hasUnsafePHIOrSelectUse(&I, Size))
         return PI.setAborted(UnsafeI);
     }
 
@@ -711,7 +703,15 @@ private:
       return;
     }
 
-    insertUse(SI, Offset, SelectSize);
+    insertUse(I, Offset, Size);
+  }
+
+  void visitPHINode(PHINode &PN) {
+    visitPHINodeOrSelectInst(PN);
+  }
+
+  void visitSelectInst(SelectInst &SI) {
+    visitPHINodeOrSelectInst(SI);
   }
 
   /// \brief Disable SROA entirely if there are unhandled users of the alloca.

test/Transforms/SROA/phi-and-select.ll

@@ -501,3 +501,68 @@ end:
 ; CHECK-NOT: load
 ; CHECK: ret float %[[phi]]
 }
+
+; Verifies we fixed PR20425. We should be able to promote all alloca's to
+; registers in this test.
+;
+; %0 = slice
+; %1 = slice
+; %2 = phi(%0, %1) // == slice
+define float @simplify_phi_nodes_that_equal_slice(i1 %cond, float* %temp) {
+; CHECK-LABEL: @simplify_phi_nodes_that_equal_slice(
+entry:
+  %arr = alloca [4 x float], align 4
+; CHECK-NOT: alloca
+  br i1 %cond, label %then, label %else
+
+then:
+  %0 = getelementptr inbounds [4 x float]* %arr, i64 0, i64 3
+  store float 1.000000e+00, float* %0, align 4
+  br label %merge
+
+else:
+  %1 = getelementptr inbounds [4 x float]* %arr, i64 0, i64 3
+  store float 2.000000e+00, float* %1, align 4
+  br label %merge
+
+merge:
+  %2 = phi float* [ %0, %then ], [ %1, %else ]
+  store float 0.000000e+00, float* %temp, align 4
+  %3 = load float* %2, align 4
+  ret float %3
+}
+
+; A slightly complicated example for PR20425.
+;
+; %0 = slice
+; %1 = phi(%0) // == slice
+; %2 = slice
+; %3 = phi(%1, %2) // == slice
+define float @simplify_phi_nodes_that_equal_slice_2(i1 %cond, float* %temp) {
+; CHECK-LABEL: @simplify_phi_nodes_that_equal_slice_2(
+entry:
+  %arr = alloca [4 x float], align 4
+; CHECK-NOT: alloca
+  br i1 %cond, label %then, label %else
+
+then:
+  %0 = getelementptr inbounds [4 x float]* %arr, i64 0, i64 3
+  store float 1.000000e+00, float* %0, align 4
+  br label %then2
+
+then2:
+  %1 = phi float* [ %0, %then ]
+  store float 2.000000e+00, float* %1, align 4
+  br label %merge
+
+else:
+  %2 = getelementptr inbounds [4 x float]* %arr, i64 0, i64 3
+  store float 3.000000e+00, float* %2, align 4
+  br label %merge
+
+merge:
+  %3 = phi float* [ %1, %then2 ], [ %2, %else ]
+  store float 0.000000e+00, float* %temp, align 4
+  %4 = load float* %3, align 4
+  ret float %4
+}