Merge pull request swiftlang#74078 from atrick/60-fix-silcombine-dead-code

[6.0] Fix SILCombine to delete dead end_access instructions.

Author: atrick

include/swift/SIL/InstructionUtils.h

@@ -107,8 +107,16 @@ SILValue stripBorrow(SILValue V);
 /// type may be changed by a cast.
 SingleValueInstruction *getSingleValueCopyOrCast(SILInstruction *I);
 
+// Return true if this instruction begins a SIL-level scope. If so, it must have
+// a single result. That result must have an isEndOfScopeMarker direct use on
+// all reachable paths. This instruction along with its scope-ending
+// instructions are considered a single operation. They must be inserted and
+// deleted together.
+bool isBeginScopeMarker(SILInstruction *user);
+
 /// Return true if this instruction terminates a SIL-level scope. Scope end
-/// instructions do not produce a result.
+/// instructions do not produce a result. Their single operand must be an
+/// isBeginScopeMarker and cannot be 'undef'.
 bool isEndOfScopeMarker(SILInstruction *user);
 
 /// Return true if the given instruction has no effect on it's operand values

lib/SIL/Utils/InstructionUtils.cpp

@@ -295,7 +295,16 @@ SingleValueInstruction *swift::getSingleValueCopyOrCast(SILInstruction *I) {
   }
 }
 
-// Does this instruction terminate a SIL-level scope?
+bool swift::isBeginScopeMarker(SILInstruction *user) {
+  switch (user->getKind()) {
+  default:
+    return false;
+  case SILInstructionKind::BeginAccessInst:
+  case SILInstructionKind::BeginBorrowInst:
+    return true;
+  }
+}
+
 bool swift::isEndOfScopeMarker(SILInstruction *user) {
   switch (user->getKind()) {
   default:

lib/SILOptimizer/SILCombiner/SILCombinerMiscVisitors.cpp

@@ -844,6 +844,15 @@ SILInstruction *SILCombiner::visitCondFailInst(CondFailInst *CFI) {
   ValueSet DefinedValues(CFI->getFunction());
   for (auto Iter = std::next(CFI->getIterator());
        Iter != CFI->getParent()->end(); ++Iter) {
+
+    if (isBeginScopeMarker(&*Iter)) {
+      for (auto *scopeUse : cast<SingleValueInstruction>(&*Iter)->getUses()) {
+        auto *scopeEnd = scopeUse->getUser();
+        if (isEndOfScopeMarker(scopeEnd)) {
+          ToRemove.push_back(scopeEnd);
+        }
+      }
+    }
     if (!CFI->getFunction()->hasOwnership()) {
       ToRemove.push_back(&*Iter);
       continue;
@@ -870,6 +879,9 @@ SILInstruction *SILCombiner::visitCondFailInst(CondFailInst *CFI) {
     return nullptr;
 
   for (auto *Inst : ToRemove) {
+    if (Inst->isDeleted())
+      continue;
+
     // Replace any still-remaining uses with undef and erase.
     Inst->replaceAllUsesOfAllResultsWithUndef();
     eraseInstFromFunction(*Inst);

test/SILOptimizer/sil_combine.sil

@@ -587,6 +587,35 @@ bb0(%0 : $Builtin.Int1):
   return %2 : $()
 }
 
+// rdar://121599876 (SILCombine should delete instructions in blocks dominated by cond_fail -1)
+// CHECK-LABEL: sil @cond_fail_end_scope : $@convention(thin) (@inout Builtin.Int32, Builtin.Int1) -> () {
+// CHECK: cond_fail
+// CHECK-NEXT:   unreachable
+// CHECK:      bb1:
+// CHECK-NEXT:   br bb3
+// CHECK:      bb2:
+// CHECK-NEXT:   br bb3
+// CHECK-LABEL: } // end sil function 'cond_fail_end_scope'
+sil @cond_fail_end_scope : $@convention(thin) (@inout Builtin.Int32, Builtin.Int1) -> () {
+entry(%0 : $*Builtin.Int32, %1 : $Builtin.Int1):
+  %true = integer_literal $Builtin.Int1, -1
+  cond_fail %true : $Builtin.Int1
+  %access = begin_access [read] [static] %0 : $*Builtin.Int32
+  cond_br %1, left, right
+
+left:
+  end_access %access : $*Builtin.Int32
+  br end
+
+right:
+  end_access %access : $*Builtin.Int32
+  br end
+
+end:
+  %99 = tuple ()
+  return %99 : $()
+}
+
 // CHECK-LABEL: sil @release_then_retain_peephole
 // CHECK: bb0
 // CHECK-NOT: strong_release
@@ -3688,7 +3717,7 @@ bb3(%16 : $Int32):                                  // Preds: bb1 bb2
   return %16 : $Int32                               // id: %17
 }
 
-protocol Prot0 : class {
+protocol Prot0 : AnyObject {
   static func newWithConfig() throws -> Builtin.Int32
 }