[rc-id] Make RCIdentity strip off single-pred arguments.

In a bunch of use-cases we use stripSinglePredecessorArgs to eliminate this
case. There is no reason to assume that this is being done in the caller of
RCIdentity. Lets make sure that we handle this case here.

rdar://24156136

Author: gottesmm

include/swift/SIL/SILArgument.h

@@ -91,6 +91,10 @@ class SILArgument : public ValueBase {
   bool getIncomingValues(
       llvm::SmallVectorImpl<std::pair<SILBasicBlock *, SILValue>> &OutArray);
 
+  /// If this SILArgument's parent block has one predecessor, return the
+  /// incoming value from that predecessor. Returns SILValue() otherwise.
+  SILValue getSingleIncomingValue() const;
+
   /// Returns true if this SILArgument is the self argument of its
   /// function. This means that this will return false always for SILArguments
   /// of SILFunctions that do not have self argument and for non-function

include/swift/SIL/SILInstruction.h

@@ -3916,11 +3916,13 @@ class CondBranchInst : public TermInst {
 
   /// Returns the argument on the cond_br terminator that will be passed to
   /// DestBB in A.
-  SILValue getArgForDestBB(SILBasicBlock *DestBB, SILArgument *A) const;
+  SILValue getArgForDestBB(const SILBasicBlock *DestBB,
+                           const SILArgument *A) const;
 
   /// Returns the argument on the cond_br terminator that will be passed as the
   /// \p Index argument to DestBB.
-  SILValue getArgForDestBB(SILBasicBlock *DestBB, unsigned ArgIndex) const;
+  SILValue getArgForDestBB(const SILBasicBlock *DestBB,
+                           unsigned ArgIndex) const;
 
   void swapSuccessors();
 

lib/SIL/SILArgument.cpp

@@ -60,9 +60,10 @@ SILModule &SILArgument::getModule() const {
   return getFunction()->getModule();
 }
 
-static SILValue getIncomingValueForPred(SILBasicBlock *BB, SILBasicBlock *Pred,
+static SILValue getIncomingValueForPred(const SILBasicBlock *BB,
+                                        const SILBasicBlock *Pred,
                                         unsigned Index) {
-  TermInst *TI = Pred->getTerminator();
+  const TermInst *TI = Pred->getTerminator();
 
   switch (TI->getTermKind()) {
   // TODO: This list is conservative. I think we can probably handle more of
@@ -77,17 +78,25 @@ static SILValue getIncomingValueForPred(SILBasicBlock *BB, SILBasicBlock *Pred,
   case TermKind::DynamicMethodBranchInst:
     return SILValue();
   case TermKind::BranchInst:
-    return cast<BranchInst>(TI)->getArg(Index);
+    return cast<const BranchInst>(TI)->getArg(Index);
   case TermKind::CondBranchInst:
-    return cast<CondBranchInst>(TI)->getArgForDestBB(BB, Index);
+    return cast<const CondBranchInst>(TI)->getArgForDestBB(BB, Index);
   case TermKind::CheckedCastBranchInst:
-    return cast<CheckedCastBranchInst>(TI)->getOperand();
+    return cast<const CheckedCastBranchInst>(TI)->getOperand();
   case TermKind::SwitchEnumInst:
-    return cast<SwitchEnumInst>(TI)->getOperand();
+    return cast<const SwitchEnumInst>(TI)->getOperand();
   }
   llvm_unreachable("Unhandled TermKind?!");
 }
 
+SILValue SILArgument::getSingleIncomingValue() const {
+  const SILBasicBlock *Parent = getParent();
+  const SILBasicBlock *PredBB = Parent->getSinglePredecessor();
+  if (!PredBB)
+    return SILValue();
+  return getIncomingValueForPred(Parent, PredBB, getIndex());
+}
+
 bool SILArgument::getIncomingValues(llvm::SmallVectorImpl<SILValue> &OutArray) {
   SILBasicBlock *Parent = getParent();
 

lib/SIL/SILInstructions.cpp

@@ -774,12 +774,12 @@ OperandValueArrayRef CondBranchInst::getFalseArgs() const {
   return Operands.asValueArray().slice(1 + NumTrueArgs, NumFalseArgs);
 }
 
-SILValue CondBranchInst::getArgForDestBB(SILBasicBlock *DestBB,
-                                         SILArgument *Arg) const {
+SILValue CondBranchInst::getArgForDestBB(const SILBasicBlock *DestBB,
+                                         const SILArgument *Arg) const {
   return getArgForDestBB(DestBB, Arg->getIndex());
 }
 
-SILValue CondBranchInst::getArgForDestBB(SILBasicBlock *DestBB,
+SILValue CondBranchInst::getArgForDestBB(const SILBasicBlock *DestBB,
                                          unsigned ArgIndex) const {
   // If TrueBB and FalseBB equal, we cannot find an arg for this DestBB so
   // return an empty SILValue.

lib/SIL/SILValue.cpp

@@ -75,6 +75,14 @@ static SILValue stripSinglePredecessorArgs(SILValue V) {
     TermInst *PredTI = Pred->getTerminator();
 
     // And attempt to find our matching argument.
+    //
+    // *NOTE* We can only strip things here if we know that there is no semantic
+    // change in terms of upcasts/downcasts/enum extraction since this is used
+    // by other routines here. This means that we can only look through
+    // cond_br/br.
+    //
+    // For instance, routines that use stripUpcasts() do not want to strip off a
+    // downcast that results from checked_cast_br.
     if (auto *BI = dyn_cast<BranchInst>(PredTI)) {
       V = BI->getArg(A->getIndex());
       continue;

lib/SILOptimizer/Analysis/RCIdentityAnalysis.cpp

@@ -98,6 +98,20 @@ static SILValue stripRCIdentityPreservingInsts(SILValue V) {
     if (SILValue NewValue = TI->getUniqueNonTrivialElt())
       return NewValue;
 
+  // Any SILArgument with a single predecessor from a "phi" perspective is
+  // dead. In such a case, the SILArgument must be rc-identical.
+  //
+  // This is the easy case. The difficult case is when you have an argument with
+  // /multiple/ predecessors.
+  //
+  // We do not need to insert this SILArgument into the visited SILArgument set
+  // since we will only visit it twice if we go around a back edge due to a
+  // different SILArgument that is actually being used for its phi node like
+  // purposes.
+  if (auto *A = dyn_cast<SILArgument>(V))
+    if (SILValue Result = A->getSingleIncomingValue())
+      return Result;
+
   return SILValue();
 }
 

test/SILOptimizer/rcidentity.sil

@@ -108,3 +108,35 @@ bb0(%0 : $Builtin.NativeObject, %1 : $Builtin.Word):
   %5 = tuple(%1 : $Builtin.Word, %4 : $(Builtin.Word, Builtin.NativeObject, Builtin.NativeObject), %1 : $Builtin.Word)
   return undef : $()
 }
+
+// All of the SSA values in the given function besides %6 can be resolved to
+// (%0, %1). Because %6 has multiple SSA values and there is nothing tricky we
+// can do here, just bail.
+//
+// CHECK-LABEL: @test_single_pred_rc_id@
+// CHECK: RESULT #0: 0 = 0
+// CHECK: RESULT #1: 1 = 1
+// CHECK: RESULT #2: 2 = 0
+// CHECK: RESULT #3: 3 = 1
+// CHECK: RESULT #4: 4 = 1
+// CHECK: RESULT #5: 5 = 0
+// CHECK: RESULT #6: 6 = 6
+sil @test_single_pred_rc_id : $@convention(thin) (Builtin.NativeObject, Builtin.NativeObject) -> () {
+bb0(%0 : $Builtin.NativeObject, %1 : $Builtin.NativeObject):
+  br bb1(%0 : $Builtin.NativeObject, %1 : $Builtin.NativeObject)
+
+bb1(%2 : $Builtin.NativeObject, %3 : $Builtin.NativeObject):
+  cond_br undef, bb2(%3 : $Builtin.NativeObject), bb3(%2 : $Builtin.NativeObject)
+
+bb2(%4 : $Builtin.NativeObject):
+  br bb4(%4 : $Builtin.NativeObject)
+
+bb3(%5 : $Builtin.NativeObject):
+  br bb4(%5 : $Builtin.NativeObject)
+
+bb4(%6 : $Builtin.NativeObject):
+  cond_br undef, bb4(%6 : $Builtin.NativeObject), bb5
+
+bb5:
+  return undef : $()
+}