Merge pull request swiftlang#35430 from gottesmm/pr-efd34fda22c8ccd45b958b606e3ff64c25edca17

gottesmm · web-flow · commit 33ab920d055d · 2021-01-14T15:12:31.000-08:00
[ownership] Eliminate Optional return value APIs from OwnershipUtils in favor of an Invalid enum case.
diff --git a/include/swift/SIL/OwnershipUtils.h b/include/swift/SIL/OwnershipUtils.h
diff --git a/lib/SIL/Utils/OwnershipUtils.cpp b/lib/SIL/Utils/OwnershipUtils.cpp
@@ -146,6 +146,8 @@ bool swift::canOpcodeForwardOwnedValues(Operand *use) {
 
 void BorrowingOperandKind::print(llvm::raw_ostream &os) const {
   switch (value) {
+  case Kind::Invalid:
+    llvm_unreachable("Using an unreachable?!");
   case Kind::BeginBorrow:
     os << "BeginBorrow";
     return;
@@ -190,6 +192,8 @@ llvm::raw_ostream &swift::operator<<(llvm::raw_ostream &os,
 bool BorrowingOperand::visitLocalEndScopeUses(
     function_ref<bool(Operand *)> func) const {
   switch (kind) {
+  case BorrowingOperandKind::Invalid:
+    llvm_unreachable("Using invalid case");
   case BorrowingOperandKind::BeginBorrow:
     for (auto *use : cast<BeginBorrowInst>(op->getUser())->getUses()) {
       if (use->isLifetimeEnding()) {
@@ -228,20 +232,24 @@ bool BorrowingOperand::visitLocalEndScopeUses(
 void BorrowingOperand::visitBorrowIntroducingUserResults(
     function_ref<void(BorrowedValue)> visitor) const {
   switch (kind) {
+  case BorrowingOperandKind::Invalid:
+    llvm_unreachable("Using invalid case");
   case BorrowingOperandKind::Apply:
   case BorrowingOperandKind::TryApply:
   case BorrowingOperandKind::BeginApply:
   case BorrowingOperandKind::Yield:
     llvm_unreachable("Never has borrow introducer results!");
   case BorrowingOperandKind::BeginBorrow: {
-    auto value = *BorrowedValue::get(cast<BeginBorrowInst>(op->getUser()));
+    auto value = BorrowedValue::get(cast<BeginBorrowInst>(op->getUser()));
+    assert(value);
     return visitor(value);
   }
   case BorrowingOperandKind::Branch: {
     auto *bi = cast<BranchInst>(op->getUser());
     for (auto *succBlock : bi->getSuccessorBlocks()) {
       auto value =
-          *BorrowedValue::get(succBlock->getArgument(op->getOperandNumber()));
+          BorrowedValue::get(succBlock->getArgument(op->getOperandNumber()));
+      assert(value);
       visitor(value);
     }
     return;
@@ -263,8 +271,8 @@ void BorrowingOperand::visitConsumingUsesOfBorrowIntroducingUserResults(
     // single guaranteed scope.
     value.visitLocalScopeEndingUses([&](Operand *valueUser) {
       if (auto subBorrowScopeOp = BorrowingOperand::get(valueUser)) {
-        if (subBorrowScopeOp->isReborrow()) {
-          subBorrowScopeOp->visitUserResultConsumingUses(func);
+        if (subBorrowScopeOp.isReborrow()) {
+          subBorrowScopeOp.visitUserResultConsumingUses(func);
           return;
         }
       }
@@ -315,6 +323,8 @@ void BorrowingOperand::getImplicitUses(
 
 void BorrowedValueKind::print(llvm::raw_ostream &os) const {
   switch (value) {
+  case BorrowedValueKind::Invalid:
+    llvm_unreachable("Using invalid case?!");
   case BorrowedValueKind::SILFunctionArgument:
     os << "SILFunctionArgument";
     return;
@@ -342,6 +352,8 @@ void BorrowedValue::getLocalScopeEndingInstructions(
   assert(isLocalScope() && "Should only call this given a local scope");
 
   switch (kind) {
+  case BorrowedValueKind::Invalid:
+    llvm_unreachable("Using invalid case?!");
   case BorrowedValueKind::SILFunctionArgument:
     llvm_unreachable("Should only call this with a local scope");
   case BorrowedValueKind::BeginBorrow:
@@ -361,6 +373,8 @@ void BorrowedValue::visitLocalScopeEndingUses(
     function_ref<void(Operand *)> visitor) const {
   assert(isLocalScope() && "Should only call this given a local scope");
   switch (kind) {
+  case BorrowedValueKind::Invalid:
+    llvm_unreachable("Using invalid case?!");
   case BorrowedValueKind::SILFunctionArgument:
     llvm_unreachable("Should only call this with a local scope");
   case BorrowedValueKind::LoadBorrow:
@@ -441,7 +455,7 @@ bool BorrowedValue::visitLocalScopeTransitiveEndingUses(
       continue;
     }
 
-    scopeOperand->visitConsumingUsesOfBorrowIntroducingUserResults(
+    scopeOperand.visitConsumingUsesOfBorrowIntroducingUserResults(
         [&](Operand *op) {
           assert(op->isLifetimeEnding() && "Expected only consuming uses");
           // Make sure we haven't visited this consuming operand yet. If we
@@ -464,7 +478,7 @@ bool BorrowedValue::visitInteriorPointerOperands(
     auto *op = worklist.pop_back_val();
 
     if (auto interiorPointer = InteriorPointerOperand::get(op)) {
-      func(*interiorPointer);
+      func(interiorPointer);
       continue;
     }
 
@@ -617,6 +631,8 @@ bool InteriorPointerOperand::getImplicitUses(
 
 void OwnedValueIntroducerKind::print(llvm::raw_ostream &os) const {
   switch (value) {
+  case OwnedValueIntroducerKind::Invalid:
+    llvm_unreachable("Using invalid case?!");
   case OwnedValueIntroducerKind::Apply:
     os << "Apply";
     return;
@@ -677,7 +693,7 @@ bool swift::getAllBorrowIntroducingValues(SILValue inputValue,
 
     // First check if v is an introducer. If so, stash it and continue.
     if (auto scopeIntroducer = BorrowedValue::get(value)) {
-      out.push_back(*scopeIntroducer);
+      out.push_back(scopeIntroducer);
       continue;
     }
 
@@ -716,10 +732,9 @@ bool swift::getAllBorrowIntroducingValues(SILValue inputValue,
   return true;
 }
 
-Optional<BorrowedValue>
-swift::getSingleBorrowIntroducingValue(SILValue inputValue) {
+BorrowedValue swift::getSingleBorrowIntroducingValue(SILValue inputValue) {
   if (inputValue.getOwnershipKind() != OwnershipKind::Guaranteed)
-    return None;
+    return {};
 
   SILValue currentValue = inputValue;
   while (true) {
@@ -738,7 +753,7 @@ swift::getSingleBorrowIntroducingValue(SILValue inputValue) {
         // this.
         auto begin = instOps.begin();
         if (std::next(begin) != instOps.end()) {
-          return None;
+          return {};
         }
         // Otherwise, set currentOp to the single operand and continue.
         currentValue = *begin;
@@ -758,7 +773,7 @@ swift::getSingleBorrowIntroducingValue(SILValue inputValue) {
 
     // Otherwise, this is an introducer we do not understand. Bail and return
     // None.
-    return None;
+    return {};
   }
 
   llvm_unreachable("Should never hit this");
@@ -777,7 +792,7 @@ bool swift::getAllOwnedValueIntroducers(
 
     // First check if v is an introducer. If so, stash it and continue.
     if (auto introducer = OwnedValueIntroducer::get(value)) {
-      out.push_back(*introducer);
+      out.push_back(introducer);
       continue;
     }
 
@@ -816,10 +831,9 @@ bool swift::getAllOwnedValueIntroducers(
   return true;
 }
 
-Optional<OwnedValueIntroducer>
-swift::getSingleOwnedValueIntroducer(SILValue inputValue) {
+OwnedValueIntroducer swift::getSingleOwnedValueIntroducer(SILValue inputValue) {
   if (inputValue.getOwnershipKind() != OwnershipKind::Owned)
-    return None;
+    return {};
 
   SILValue currentValue = inputValue;
   while (true) {
@@ -838,7 +852,7 @@ swift::getSingleOwnedValueIntroducer(SILValue inputValue) {
         // this.
         auto begin = instOps.begin();
         if (std::next(begin) != instOps.end()) {
-          return None;
+          return {};
         }
         // Otherwise, set currentOp to the single operand and continue.
         currentValue = *begin;
@@ -859,7 +873,7 @@ swift::getSingleOwnedValueIntroducer(SILValue inputValue) {
 
     // Otherwise, this is an introducer we do not understand. Bail and return
     // None.
-    return None;
+    return {};
   }
 
   llvm_unreachable("Should never hit this");
@@ -869,12 +883,12 @@ swift::getSingleOwnedValueIntroducer(SILValue inputValue) {
 //                             Forwarding Operand
 //===----------------------------------------------------------------------===//
 
-Optional<ForwardingOperand> ForwardingOperand::get(Operand *use) {
+ForwardingOperand ForwardingOperand::get(Operand *use) {
   if (use->isTypeDependent())
-    return None;
+    return nullptr;
 
   if (!OwnershipForwardingMixin::isa(use->getUser())) {
-    return None;
+    return nullptr;
   }
 #ifndef NDEBUG
   switch (use->getOperandOwnership()) {
diff --git a/lib/SIL/Verifier/LinearLifetimeChecker.cpp b/lib/SIL/Verifier/LinearLifetimeChecker.cpp
@@ -314,7 +314,7 @@ void State::checkForSameBlockUseAfterFree(Operand *consumingUse,
         continue;
       }
     } else if (auto borrowingOperand = BorrowingOperand::get(consumingUse)) {
-      assert(borrowingOperand->isReborrow());
+      assert(borrowingOperand.isReborrow());
       continue;
     }
 
diff --git a/lib/SIL/Verifier/ReborrowVerifier.cpp b/lib/SIL/Verifier/ReborrowVerifier.cpp
@@ -54,7 +54,7 @@ void ReborrowVerifier::verifyReborrows(BorrowingOperand initialScopedOperand,
     auto *borrowLifetimeEndUser = borrowLifetimeEndOp->getUser();
 
     auto borrowingOperand = BorrowingOperand::get(borrowLifetimeEndOp);
-    if (!borrowingOperand || !borrowingOperand->isReborrow())
+    if (!borrowingOperand || !borrowingOperand.isReborrow())
       continue;
 
     if (isVisitedOp(borrowLifetimeEndOp, baseVal))
@@ -85,8 +85,8 @@ void ReborrowVerifier::verifyReborrows(BorrowingOperand initialScopedOperand,
       // Find the scope ending uses of the guaranteed phi arg and add it to the
       // worklist.
       auto scopedValue = BorrowedValue::get(phiArg);
-      assert(scopedValue.hasValue());
-      scopedValue->visitLocalScopeEndingUses([&](Operand *op) {
+      assert(scopedValue);
+      scopedValue.visitLocalScopeEndingUses([&](Operand *op) {
         addVisitedOp(op, newBaseVal);
         worklist.emplace_back(op, newBaseVal);
       });
diff --git a/lib/SIL/Verifier/SILOwnershipVerifier.cpp b/lib/SIL/Verifier/SILOwnershipVerifier.cpp
@@ -266,8 +266,8 @@ bool SILValueOwnershipChecker::gatherNonGuaranteedUsers(
                      << "Initial: " << *initialScopedOperand << "\n";
       });
     };
-    initialScopedOperand->getImplicitUses(nonLifetimeEndingUsers, &error);
-    reborrowVerifier.verifyReborrows(initialScopedOperand.getValue(), value);
+    initialScopedOperand.getImplicitUses(nonLifetimeEndingUsers, &error);
+    reborrowVerifier.verifyReborrows(initialScopedOperand, value);
   }
 
   return foundError;
@@ -354,7 +354,7 @@ bool SILValueOwnershipChecker::gatherUsers(
       // BorrowScopeOperand and if so, add its end scope instructions as
       // implicit regular users of our value.
       if (auto scopedOperand = BorrowingOperand::get(op)) {
-        assert(!scopedOperand->isReborrow());
+        assert(!scopedOperand.isReborrow());
 
         std::function<void(Operand *)> onError = [&](Operand *op) {
           errorBuilder.handleMalformedSIL([&] {
@@ -364,8 +364,8 @@ bool SILValueOwnershipChecker::gatherUsers(
           });
         };
 
-        scopedOperand->getImplicitUses(nonLifetimeEndingUsers, &onError);
-        reborrowVerifier.verifyReborrows(scopedOperand.getValue(), value);
+        scopedOperand.getImplicitUses(nonLifetimeEndingUsers, &onError);
+        reborrowVerifier.verifyReborrows(scopedOperand, value);
       }
 
       // Next see if our use is an interior pointer operand. If we have an
@@ -377,11 +377,11 @@ bool SILValueOwnershipChecker::gatherUsers(
             llvm::errs() << "Could not recognize address user of interior "
                             "pointer operand!\n"
                          << "Interior Pointer Operand: "
-                         << *interiorPointerOperand->operand->getUser()
+                         << *interiorPointerOperand.operand->getUser()
                          << "Address User: " << *op->getUser();
           });
         };
-        foundError |= interiorPointerOperand->getImplicitUses(
+        foundError |= interiorPointerOperand.getImplicitUses(
             nonLifetimeEndingUsers, &onError);
       }
 
diff --git a/lib/SILOptimizer/SemanticARC/CopyValueOpts.cpp b/lib/SILOptimizer/SemanticARC/CopyValueOpts.cpp
@@ -334,7 +334,7 @@ static Operand *lookThroughSingleForwardingUse(Operand *use) {
   auto forwardingOperand = ForwardingOperand::get(use);
   if (!forwardingOperand)
     return nullptr;
-  auto forwardedValue = (*forwardingOperand).getSingleForwardedValue();
+  auto forwardedValue = forwardingOperand.getSingleForwardedValue();
   if (!forwardedValue)
     return nullptr;
   auto *singleConsumingUse = forwardedValue->getSingleConsumingUse();
@@ -426,7 +426,7 @@ static bool tryJoinIfDestroyConsumingUseInSameBlock(
     auto forwardingOperand = ForwardingOperand::get(currentForwardingUse);
     if (!forwardingOperand)
       return false;
-    auto forwardedValue = (*forwardingOperand).getSingleForwardedValue();
+    auto forwardedValue = forwardingOperand.getSingleForwardedValue();
     if (!forwardedValue)
       return false;
 
@@ -478,7 +478,7 @@ static bool tryJoinIfDestroyConsumingUseInSameBlock(
     // singleConsumingUse (the original forwarded use) and the destroy_value. In
     // such a case, we must bail!
     if (auto operand = BorrowingOperand::get(use))
-      if (!operand->visitLocalEndScopeUses([&](Operand *endScopeUse) {
+      if (!operand.visitLocalEndScopeUses([&](Operand *endScopeUse) {
             // Return false if we did see the relevant end scope instruction
             // in the block. That means that we are going to exit early and
             // return false.
diff --git a/lib/SILOptimizer/SemanticARC/LoadCopyToLoadBorrowOpt.cpp b/lib/SILOptimizer/SemanticARC/LoadCopyToLoadBorrowOpt.cpp
@@ -221,8 +221,8 @@ class StorageGuaranteesLoadVisitor
     // know statically that our let can not be written to in the current
     // function. To be conservative, assume that all other non-local scopes
     // write to memory.
-    if (!value->isLocalScope()) {
-      if (value->kind == BorrowedValueKind::SILFunctionArgument) {
+    if (!value.isLocalScope()) {
+      if (value.kind == BorrowedValueKind::SILFunctionArgument) {
         return answer(false);
       }
 
@@ -233,7 +233,7 @@ class StorageGuaranteesLoadVisitor
 
     // TODO: This is disabled temporarily for guaranteed phi args just for
     // staging purposes. Thus be conservative and assume true in these cases.
-    if (value->kind == BorrowedValueKind::Phi) {
+    if (value.kind == BorrowedValueKind::Phi) {
       return answer(true);
     }
 
@@ -243,7 +243,7 @@ class StorageGuaranteesLoadVisitor
     // to check whether the copied value is dominated by the lifetime of the
     // borrow it's based on.
     SmallVector<Operand *, 4> endScopeInsts;
-    value->visitLocalScopeEndingUses(
+    value.visitLocalScopeEndingUses(
         [&](Operand *use) { endScopeInsts.push_back(use); });
 
     SmallPtrSet<SILBasicBlock *, 4> visitedBlocks;
diff --git a/lib/SILOptimizer/SemanticARC/OwnedToGuaranteedPhiOpt.cpp b/lib/SILOptimizer/SemanticARC/OwnedToGuaranteedPhiOpt.cpp
@@ -61,19 +61,19 @@ static bool canEliminatePhi(
     // to eliminate. Since we do not look through joined live ranges, we must
     // only have a single introducer. So look for that one and if not, bail.
     auto singleIntroducer = getSingleOwnedValueIntroducer(incomingValue);
-    if (!singleIntroducer.hasValue()) {
+    if (!singleIntroducer) {
       return false;
     }
 
     // Then make sure that our owned value introducer is able to be converted to
     // guaranteed and that we found it to have a LiveRange that we could have
     // eliminated /if/ we were to get rid of this phi.
-    if (!singleIntroducer->isConvertableToGuaranteed()) {
+    if (!singleIntroducer.isConvertableToGuaranteed()) {
       return false;
     }
 
     // Otherwise, add the introducer to our result array.
-    ownedValueIntroducerAccumulator.push_back(*singleIntroducer);
+    ownedValueIntroducerAccumulator.push_back(singleIntroducer);
   }
 
 #ifndef NDEBUG
diff --git a/lib/SILOptimizer/SemanticARC/OwnershipLiveRange.cpp b/lib/SILOptimizer/SemanticARC/OwnershipLiveRange.cpp
@@ -19,8 +19,9 @@ using namespace swift;
 using namespace swift::semanticarc;
 
 OwnershipLiveRange::OwnershipLiveRange(SILValue value)
-    : introducer(*OwnedValueIntroducer::get(value)), destroyingUses(),
+    : introducer(OwnedValueIntroducer::get(value)), destroyingUses(),
       ownershipForwardingUses(), unknownConsumingUses() {
+  assert(introducer);
   assert(introducer.value.getOwnershipKind() == OwnershipKind::Owned);
 
   SmallVector<Operand *, 32> tmpDestroyingUses;
@@ -227,7 +228,7 @@ void OwnershipLiveRange::convertOwnedGeneralForwardingUsesToGuaranteed() && {
   while (!ownershipForwardingUses.empty()) {
     auto *use = ownershipForwardingUses.back();
     ownershipForwardingUses = ownershipForwardingUses.drop_back();
-    auto operand = *ForwardingOperand::get(use);
+    auto operand = ForwardingOperand::get(use);
     operand.replaceOwnershipKind(OwnershipKind::Owned,
                                  OwnershipKind::Guaranteed);
   }
@@ -253,6 +254,8 @@ void OwnershipLiveRange::convertToGuaranteedAndRAUW(
 // TODO: If this is useful, move onto OwnedValueIntroducer itself?
 static SILValue convertIntroducerToGuaranteed(OwnedValueIntroducer introducer) {
   switch (introducer.kind) {
+  case OwnedValueIntroducerKind::Invalid:
+    llvm_unreachable("Using invalid case?!");
   case OwnedValueIntroducerKind::Phi: {
     auto *phiArg = cast<SILPhiArgument>(introducer.value);
     phiArg->setOwnershipKind(OwnershipKind::Guaranteed);
diff --git a/lib/SILOptimizer/Utils/CanonicalOSSALifetime.cpp b/lib/SILOptimizer/Utils/CanonicalOSSALifetime.cpp
diff --git a/lib/SILOptimizer/Utils/OwnershipOptUtils.cpp b/lib/SILOptimizer/Utils/OwnershipOptUtils.cpp

Original file line number	Diff line number	Diff line change
`@@ -314,7 +314,7 @@ void State::checkForSameBlockUseAfterFree(Operand *consumingUse,`
`314`	`314`	`continue;`
`315`	`315`	`}`
`316`	`316`	`} else if (auto borrowingOperand = BorrowingOperand::get(consumingUse)) {`
`317`		`- assert(borrowingOperand->isReborrow());`
	`317`	`+ assert(borrowingOperand.isReborrow());`
`318`	`318`	`continue;`
`319`	`319`	`}`
`320`	`320`