SIL: remove computeIsReborrow

eeckstein · eeckstein · commit 7bf0010c9283 · 2024-11-12T09:26:59.000+01:00
This API computes the re-borrow flags for guaranteed phis based on the presence of forwarding instructions in the incoming values.
This is not correct in all cases, because optimizations can optimize away forwarding instructions.

Fixes a verifier crash: rdar://139280579
diff --git a/include/swift/SIL/OwnershipUtils.h b/include/swift/SIL/OwnershipUtils.h
@@ -57,14 +57,6 @@ bool canOpcodeForwardInnerGuaranteedValues(SILValue value);
 /// the operation may be trivially rewritten with Guaranteed ownership.
 bool canOpcodeForwardInnerGuaranteedValues(Operand *use);
 
-bool computeIsScoped(SILArgument *arg);
-
-bool computeIsReborrow(SILArgument *arg);
-
-// This is the use-def equivalent of use->getOperandOwnership() ==
-// OperandOwnership::GuaranteedForwarding.
-bool computeIsGuaranteedForwarding(SILValue value);
-
 /// Is the opcode that produces \p value capable of forwarding owned values?
 ///
 /// This may be true even if the current instance of the instruction is not a
diff --git a/lib/SIL/Parser/ParseSIL.cpp b/lib/SIL/Parser/ParseSIL.cpp
@@ -65,11 +65,6 @@ static llvm::cl::opt<bool>
 ParseIncompleteOSSA("parse-incomplete-ossa",
                     llvm::cl::desc("Parse OSSA with incomplete lifetimes"));
 
-static llvm::cl::opt<bool> DisablePopulateOwnershipFlags(
-    "disable-populate-ownership-flags",
-    llvm::cl::desc("Disable populating ownership flags"),
-    llvm::cl::init(false));
-
 //===----------------------------------------------------------------------===//
 // SILParserState implementation
 //===----------------------------------------------------------------------===//
@@ -7237,16 +7232,6 @@ bool SILParser::parseSILBasicBlock(SILBuilder &B) {
   return false;
 }
 
-static void populateOwnershipFlags(SILFunction *func) {
-  for (auto &bb : *func) {
-    for (auto &arg : bb.getArguments()) {
-      if (computeIsReborrow(arg)) {
-        arg->setReborrow(true);
-      }
-    }
-  }
-}
-
 ///   decl-sil:   [[only in SIL mode]]
 ///     'sil' sil-linkage '@' identifier ':' sil-type decl-sil-body?
 ///   decl-sil-body:
@@ -7426,13 +7411,6 @@ bool SILParserState::parseDeclSIL(Parser &P) {
             return true;
         } while (P.Tok.isNot(tok::r_brace) && P.Tok.isNot(tok::eof));
 
-        // OSSA uses flags to represent "reborrow", "escaping" etc.
-        // These flags are populated here as a shortcut to rewriting all
-        // existing OSSA tests with flags.
-        if (!DisablePopulateOwnershipFlags) {
-          populateOwnershipFlags(FunctionState.F);
-        }
-
         SourceLoc RBraceLoc;
         P.parseMatchingToken(tok::r_brace, RBraceLoc, diag::expected_sil_rbrace,
                              LBraceLoc);
diff --git a/lib/SIL/Utils/OwnershipUtils.cpp b/lib/SIL/Utils/OwnershipUtils.cpp
@@ -146,64 +146,6 @@ bool swift::canOpcodeForwardOwnedValues(Operand *use) {
   return false;
 }
 
-bool swift::computeIsReborrow(SILArgument *arg) {
-  if (arg->getOwnershipKind() != OwnershipKind::Guaranteed) {
-    return false;
-  }
-  if (isa<SILFunctionArgument>(arg)) {
-    return false;
-  }
-  return !computeIsGuaranteedForwarding(arg);
-}
-
-bool swift::computeIsScoped(SILArgument *arg) {
-  if (arg->getOwnershipKind() == OwnershipKind::Owned) {
-    return true;
-  }
-  return computeIsReborrow(arg);
-}
-
-// This is the use-def equivalent of use->getOperandOwnership() ==
-// OperandOwnership::GuaranteedForwarding.
-bool swift::computeIsGuaranteedForwarding(SILValue value) {
-  if (value->getOwnershipKind() != OwnershipKind::Guaranteed) {
-    return false;
-  }
-  value = lookThroughBorrowedFromDef(value);
-  // NOTE: canOpcodeForwardInnerGuaranteedValues returns true for transformation
-  // terminator results.
-  if (canOpcodeForwardInnerGuaranteedValues(value) ||
-      isa<SILFunctionArgument>(value)) {
-    return true;
-  }
-  // If not a phi, return false
-  auto *phi = dyn_cast<SILPhiArgument>(value);
-  if (!phi || !phi->isPhi()) {
-    return false;
-  }
-  // For a phi, if we find GuaranteedForwarding phi operand on any incoming
-  // path, we return true. Additional verification is added to ensure
-  // GuaranteedForwarding phi operands are found on zero or all paths in the
-  // OwnershipVerifier.
-  bool isGuaranteedForwardingPhi = false;
-  phi->visitTransitiveIncomingPhiOperands([&](auto *, auto *op) -> bool {
-    assert(op->get()->getOwnershipKind().isCompatibleWith(
-        OwnershipKind::Guaranteed));
-    auto opValue = lookThroughBorrowedFromDef(op->get());
-    if (canOpcodeForwardInnerGuaranteedValues(opValue) ||
-        isa<SILFunctionArgument>(opValue)) {
-      isGuaranteedForwardingPhi = true;
-      return false;
-    }
-    auto *phi = dyn_cast<SILPhiArgument>(opValue);
-    if (!phi || !phi->isPhi()) {
-      return false;
-    }
-    return true;
-  });
-  return isGuaranteedForwardingPhi;
-}
-
 BorrowedFromInst *swift::getBorrowedFromUser(SILValue v) {
   for (auto *use : v->getUses()) {
     if (auto *bfi = dyn_cast<BorrowedFromInst>(use->getUser())) {
diff --git a/lib/SIL/Verifier/SILVerifier.cpp b/lib/SIL/Verifier/SILVerifier.cpp
@@ -1277,11 +1277,6 @@ class SILVerifier : public SILVerifierBase<SILVerifier> {
     CurArgument = arg;
     checkLegalType(arg->getFunction(), arg, nullptr);
 
-    // Ensure flags on the argument are not stale.
-    require(!arg->getFunction()->hasOwnership() ||
-                computeIsReborrow(arg) == arg->isReborrow(),
-            "Stale reborrow flag");
-
     if (checkLinearLifetime) {
       checkValueBaseOwnership(arg);
     }
diff --git a/lib/SILOptimizer/SemanticARC/OwnershipLiveRange.cpp b/lib/SILOptimizer/SemanticARC/OwnershipLiveRange.cpp
@@ -281,7 +281,6 @@ static SILValue convertIntroducerToGuaranteed(OwnedValueIntroducer introducer) {
   case OwnedValueIntroducerKind::Phi: {
     auto *phiArg = cast<SILPhiArgument>(introducer.value);
     phiArg->setOwnershipKind(OwnershipKind::Guaranteed);
-    phiArg->setReborrow(computeIsReborrow(phiArg));
     return phiArg;
   }
   case OwnedValueIntroducerKind::Struct: {
diff --git a/lib/SILOptimizer/Utils/SILSSAUpdater.cpp b/lib/SILOptimizer/Utils/SILSSAUpdater.cpp
@@ -247,8 +247,6 @@ SILValue SILSSAUpdater::getValueInMiddleOfBlock(SILBasicBlock *block) {
     addNewEdgeValueToBranch(pair.first->getTerminator(), block, pair.second,
                             deleter);
   }
-  // Set the reborrow flag on the newly created phi.
-  phiArg->setReborrow(computeIsReborrow(phiArg));
 
   if (insertedPhis)
     insertedPhis->push_back(phiArg);
@@ -359,9 +357,6 @@ class SSAUpdaterTraits<SILSSAUpdater> {
     auto *ti = predBlock->getTerminator();
 
     changeEdgeValue(ti, phiBlock, phiArgIndex, value);
-
-    // Set the reborrow flag.
-    phi->setReborrow(computeIsReborrow(phi));
   }
 
   /// Check if an instruction is a PHI.
diff --git a/test/SIL/OwnershipVerifier/arguments.sil b/test/SIL/OwnershipVerifier/arguments.sil
@@ -38,7 +38,7 @@ bb1(%1 : @guaranteed $Builtin.NativeObject):
 
 // CHECK-LABEL: Error#: 0. Begin Error in Function: 'no_end_borrow_error2'
 // CHECK: Non trivial values, non address values, and non guaranteed function args must have at least one lifetime ending use?!
-// CHECK: Value: %3 = argument of bb1 : $Builtin.NativeObject
+// CHECK: Value: %1 = begin_borrow %0 : $Builtin.NativeObject
 // CHECK-LABEL: Error#: 0. End Error in Function: 'no_end_borrow_error2'
 //
 // CHECK-NOT: Error#: {{[0-9][0-9]*}}. End Error in Function: 'no_end_borrow_error2'
@@ -272,8 +272,8 @@ bb6:
 // CHECK: User:   end_borrow %5 : $Builtin.NativeObject           // id: %13
 // CHECK: Block: bb2
 // CHECK: Consuming Users:
-// CHECK:   end_borrow %5 : $Builtin.NativeObject           // id: %13
-// CHECK:   end_borrow %5 : $Builtin.NativeObject           // id: %7
+// CHECK:   end_borrow %5 : $Builtin.NativeObject
+// CHECK:   end_borrow %5 : $Builtin.NativeObject
 // CHECK: Error#: 6. End Error in Function: 'bad_order_add_a_level'
 //
 // CHECK-LABEL: Error#: 7. Begin Error in Function: 'bad_order_add_a_level'
@@ -282,36 +282,18 @@ bb6:
 // CHECK: Post Dominating Failure Blocks:
 // CHECK: bb3
 // CHECK: Error#: 7. End Error in Function: 'bad_order_add_a_level'
-
-// CHECK: Error#: 9. Begin Error in Function: 'bad_order_add_a_level'
-// CHECK: Error! Found a leak due to a consuming post-dominance failure!
-// CHECK: Value: %5 = argument of bb2 : $Builtin.NativeObject      // users: %13, %9, %7
-// CHECK: Post Dominating Failure Blocks:
-// CHECK: bb3
-// CHECK-LABEL: Error#: 9. End Error in Function: 'bad_order_add_a_level'
 //
-// CHECK-LABEL: Error#: 10. Begin Error in Function: 'bad_order_add_a_level'
+// CHECK-LABEL: Error#: 9. Begin Error in Function: 'bad_order_add_a_level'
 // CHECK: Found outside of lifetime use!
 // CHECK: Value: %5 = argument of bb2 : $Builtin.NativeObject      // users: %13, %9, %7
-// CHECK: User:  br bb4(%9 : $Builtin.NativeObject)              // id: %10
+// CHECK: User:  %9 = begin_borrow %5 : $Builtin.NativeObject
 // CHECK: Block: bb3
-// CHECK-LABEL: Error#: 10. End Error in Function: 'bad_order_add_a_level'
-//
-// We found a use that we didn't visit. For our purposes, we consider this to be
-// an outside lifetime use rather than use after freer to unite it with errors
-// from uses that are not reachable from the definition of the value.
-//
-// CHECK-LABEL: Error#: 11. Begin Error in Function: 'bad_order_add_a_level'
-// CHECK: Found outside of lifetime use!
-// CHECK: Value: %5 = argument of bb2 : $Builtin.NativeObject      // users: %13, %9, %7
-// CHECK: User:  %9 = begin_borrow %5 : $Builtin.NativeObject    // user: %10
-// CHECK: Block: bb3
-// CHECK: Error#: 11. End Error in Function: 'bad_order_add_a_level'
+// CHECK-LABEL: Error#: 9. End Error in Function: 'bad_order_add_a_level'
 //
-// CHECK-LABEL: Error#: 12. Begin Error in Function: 'bad_order_add_a_level'
+// CHECK-LABEL: Error#: 10. Begin Error in Function: 'bad_order_add_a_level'
 // CHECK: Non trivial values, non address values, and non guaranteed function args must have at least one lifetime ending use?!
-// CHECK: Value: %11 = argument of bb4 : $Builtin.NativeObject
-// CHECK: Error#: 12. End Error in Function: 'bad_order_add_a_level'
+// CHECK: Value: %9 = begin_borrow %5 : $Builtin.NativeObject
+// CHECK: Error#: 10. End Error in Function: 'bad_order_add_a_level'
 sil [ossa] @bad_order_add_a_level : $@convention(thin) (@owned Builtin.NativeObject) -> () {
 bb0(%0 : @owned $Builtin.NativeObject):
   %1 = begin_borrow %0 : $Builtin.NativeObject
diff --git a/test/SILOptimizer/borrow_introducer_unit.sil b/test/SILOptimizer/borrow_introducer_unit.sil
@@ -126,21 +126,16 @@ none(%phi : @guaranteed $FakeOptional<C>):
   return %retval : $()
 }
 
-// An unreachable phi currently looks like a reborrow.
-//
-// TODO: When we have a reborrow flag, an unreachable phi can be
-// either a reborrow or forwarded, as long as it has no end_borrow.
+// An unreachable phi can be either a reborrow or forwarded, as long as it has no end_borrow.
 
 // CHECK-LABEL: introducer_unreachable: find_borrow_introducers with: %phiCycle
 // CHECK: } // end sil function 'introducer_unreachable'
 // CHECK: Introducers:
-// CHECK-NEXT: argument of bb1 : $C
 // CHECK-NEXT: introducer_unreachable: find_borrow_introducers with: %phiCycle
 
 // CHECK-LABEL: introducer_unreachable: borrow_introducers with: %phiCycle
 // CHECK: } // end sil function 'introducer_unreachable'
 // CHECK: Borrow introducers for: %1 = argument of bb1 : $C
-// CHECK-NEXT: argument of bb1 : $C
 // CHECK-NEXT: introducer_unreachable: borrow_introducers with: %phiCycle
 sil [ossa] @introducer_unreachable : $@convention(thin) () -> () {
 entry:
diff --git a/test/SILOptimizer/borrowed_from_updater.sil b/test/SILOptimizer/borrowed_from_updater.sil
@@ -64,13 +64,8 @@ bb1(%phi : @guaranteed $FakeOptional<C>):
   return %retval : $()
 }
 
-// An unreachable phi currently looks like a reborrow.
-//
-// TODO: When we have a reborrow flag, an unreachable phi can be
-// either a reborrow or forwarded, as long as it has no end_borrow.
-
 // CHECK-LABEL: sil [ossa] @introducer_unreachable : $@convention(thin) () -> () {
-// CHECK:       bb1([[A:%.*]] : @reborrow @guaranteed $C):
+// CHECK:       bb1([[A:%.*]] : @guaranteed $C):
 // CHECK-NEXT:    [[B:%.*]] = borrowed [[A]] : $C from ()
 // CHECK:       } // end sil function 'introducer_unreachable'
 sil [ossa] @introducer_unreachable : $@convention(thin) () -> () {
diff --git a/test/SILOptimizer/enclosing_def_unit.sil b/test/SILOptimizer/enclosing_def_unit.sil
@@ -222,18 +222,19 @@ bb3(%reborrow_inner3 : @guaranteed $C):
 
 // CHECK-LABEL: enclosing_def_cycle: find_enclosing_defs with: %forward
 // CHECK: sil [ossa] @enclosing_def_cycle : $@convention(thin) (@guaranteed C) -> () {
-// CHECK: bb1([[REBORROW:%.*]] : @reborrow @guaranteed $C,
+// CHECK: bb1([[REBORROW:%.*]] : @guaranteed $C,
 // CHECK: } // end sil function 'enclosing_def_cycle'
 // CHECK: Enclosing Defs:
 // CHECK-NEXT: [[REBORROW]] = argument of bb1 : $C
 // CHECK-NEXT: enclosing_def_cycle: find_enclosing_defs with: %forward
 
 // CHECK-LABEL: enclosing_def_cycle: enclosing_values with: %forward
 // CHECK: sil [ossa] @enclosing_def_cycle : $@convention(thin) (@guaranteed C) -> () {
-// CHECK: bb1([[REBORROW:%.*]] : @reborrow @guaranteed $C,
+// CHECK: bb1([[REBORROW:%.*]] : @guaranteed $C,
 // CHECK: } // end sil function 'enclosing_def_cycle'
 // CHECK: Enclosing values for: %{{.*}} = argument of bb1 : $C
-// CHECK-NEXT: [[REBORROW]] = argument of bb1 : $C
+// CHECK-NEXT: %1 = begin_borrow %0
+// CHECK-NEXT: %0 = argument of bb0
 // CHECK-NEXT: enclosing_def_cycle: enclosing_values with: %forward
 sil [ossa] @enclosing_def_cycle : $@convention(thin) (@guaranteed C) -> () {
 entry(%0 : @guaranteed $C):
diff --git a/test/SILOptimizer/ownership_liveness_unit.sil b/test/SILOptimizer/ownership_liveness_unit.sil
@@ -1195,7 +1195,7 @@ bb2:
   %d2 = unchecked_ref_cast %0 : $C to $D
   br bb3(%copy2 : $C, %borrow2 : $C, %d2 :$D)
 
-bb3(%outer3 : @owned $C, %inner3 : @guaranteed $C, %phi3 : @guaranteed $D):
+bb3(%outer3 : @owned $C, %inner3 : @reborrow @guaranteed $C, %phi3 : @guaranteed $D):
   specify_test "interior-liveness %inner3"
   specify_test "interior_liveness_swift %inner3"
   br bb4(%phi3 : $D)
@@ -1245,7 +1245,7 @@ bb2:
   %d2 = unchecked_ref_cast %borrow2 : $C to $D
   br bb3(%copy2 : $C, %borrow2 : $C, %d2 :$D)
 
-bb3(%outer3 : @owned $C, %inner3 : @guaranteed $C, %phi3 : @guaranteed $D):
+bb3(%outer3 : @owned $C, %inner3 : @reborrow @guaranteed $C, %phi3 : @guaranteed $D):
   specify_test "interior-liveness %inner3"
   specify_test "interior_liveness_swift %inner3"
   br bb4(%phi3 : $D)
diff --git a/test/SILOptimizer/simplify_cfg_dom_jumpthread.sil b/test/SILOptimizer/simplify_cfg_dom_jumpthread.sil
@@ -49,6 +49,13 @@ enum E {
 	case C
 }
 
+enum E2 {
+  case a
+  case b(C)
+  case c(C)
+}
+
+
 class Base { }
 
 class Derived1 : Base { }
@@ -431,3 +438,62 @@ bb5(%a3 : $Builtin.Int64):
   return %a3 : $Builtin.Int64
 }
 
+// CHECK-LABEL: sil [ossa] @test_reborrow_flag1 :
+// CHECK-NOT:     @reborrow
+// CHECK:       } // end sil function 'test_reborrow_flag1'
+sil [ossa] @test_reborrow_flag1 : $@convention(thin) (@guaranteed E2) -> () {
+bb0(%0 : @guaranteed $E2):
+  %1 = begin_borrow %0 : $E2
+  switch_enum %1 : $E2, case #E2.b!enumelt: bb1, default bb2
+
+bb1(%3 : @guaranteed $C):
+  br bb3
+
+bb2(%5 : @guaranteed $E2):
+  br bb3
+
+bb3:
+  switch_enum %1 : $E2, case #E2.c!enumelt: bb4, default bb5
+
+bb4(%8 : @guaranteed $C):
+  end_borrow %1 : $E2
+  br bb6
+
+bb5(%11 : @guaranteed $E2):
+  end_borrow %1 : $E2
+  br bb6
+
+bb6:
+  %r = tuple ()
+  return %r : $()
+}
+
+// CHECK-LABEL: sil [ossa] @test_reborrow_flag2 :
+// CHECK:         @reborrow
+// CHECK:       } // end sil function 'test_reborrow_flag2'
+sil [ossa] @test_reborrow_flag2 : $@convention(thin) (@guaranteed E2, @in_guaranteed E2) -> () {
+bb0(%0 : @guaranteed $E2, %1 : $*E2):
+  switch_enum %0 : $E2, case #E2.b!enumelt: bb1, default bb2
+
+bb1(%3 : @guaranteed $C):
+  br bb3
+
+bb2(%5 : @guaranteed $E2):
+  br bb3
+
+bb3:
+  %6 = load_borrow %1 : $*E2
+  switch_enum %0 : $E2, case #E2.c!enumelt: bb4, default bb5
+
+bb4(%8 : @guaranteed $C):
+  br bb6
+
+bb5(%11 : @guaranteed $E2):
+  br bb6
+
+bb6:
+  fix_lifetime %6 : $E2
+  end_borrow %6 : $E2
+  %r = tuple ()
+  return %r : $()
+}

Original file line number	Diff line number	Diff line change
`@@ -281,7 +281,6 @@ static SILValue convertIntroducerToGuaranteed(OwnedValueIntroducer introducer) {`
`281`	`281`	`case OwnedValueIntroducerKind::Phi: {`
`282`	`282`	`auto *phiArg = cast<SILPhiArgument>(introducer.value);`
`283`	`283`	`phiArg->setOwnershipKind(OwnershipKind::Guaranteed);`
`284`		`- phiArg->setReborrow(computeIsReborrow(phiArg));`
`285`	`284`	`return phiArg;`
`286`	`285`	`}`
`287`	`286`	`case OwnedValueIntroducerKind::Struct: {`