From 03e2e8f476e963dc6c911ca7b585f15d8e5053d6 Mon Sep 17 00:00:00 2001 From: Joe Groff Date: Wed, 31 Jan 2024 16:09:53 -0800 Subject: [PATCH 1/2] BorrowToDestructureUtils: handle borrowing switches. Instead of trying to convert all switches into consumes, allow a switch of a borrow to remain as such. --- .../Utils/FieldSensitivePrunedLiveness.cpp | 12 +++ lib/SILGen/SILGenPattern.cpp | 93 +++++++++++-------- .../MoveOnlyBorrowToDestructureUtils.cpp | 54 ++++++++++- .../Mandatory/MoveOnlyObjectCheckerUtils.cpp | 8 +- 4 files changed, 120 insertions(+), 47 deletions(-) diff --git a/lib/SIL/Utils/FieldSensitivePrunedLiveness.cpp b/lib/SIL/Utils/FieldSensitivePrunedLiveness.cpp index 15e9ca0914023..0da5861a02ec3 100644 --- a/lib/SIL/Utils/FieldSensitivePrunedLiveness.cpp +++ b/lib/SIL/Utils/FieldSensitivePrunedLiveness.cpp @@ -304,11 +304,23 @@ SubElementOffset::computeForValue(SILValue projectionDerivedFromRoot, // So our payload is always going to start at the current field number since // we are the left most child of our parent enum. So we just need to look // through to our parent enum. + // + // Enum projections can happen either directly via an unchecked instruction… if (auto *enumData = dyn_cast(projectionDerivedFromRoot)) { projectionDerivedFromRoot = enumData->getOperand(); continue; } + + // …or via the bb arg of a `switch_enum` successor. + if (auto bbArg = dyn_cast(projectionDerivedFromRoot)) { + if (auto pred = bbArg->getParent()->getSinglePredecessorBlock()) { + if (auto switchEnum = dyn_cast(pred->getTerminator())) { + projectionDerivedFromRoot = switchEnum->getOperand(); + continue; + } + } + } // If we do not know how to handle this case, just return None. // diff --git a/lib/SILGen/SILGenPattern.cpp b/lib/SILGen/SILGenPattern.cpp index 7cbcfdee761c4..a54dc77531d0d 100644 --- a/lib/SILGen/SILGenPattern.cpp +++ b/lib/SILGen/SILGenPattern.cpp @@ -523,7 +523,8 @@ class PatternMatchEmission { const FailureHandler &failure); void emitGuardBranch(SILLocation loc, Expr *guard, - const FailureHandler &failure); + const FailureHandler &failure, + const ClauseRow &row, ArgArray args); // Bind copyable variable bindings as independent variables. void bindIrrefutablePatterns(const ClauseRow &row, ArgArray args, @@ -1183,25 +1184,21 @@ void PatternMatchEmission::emitWildcardDispatch(ClauseMatrix &clauses, if (auto ownership = getNoncopyableOwnership()) { // A noncopyable pattern match always happens over a borrow first. - // If there is a guard expression, then we also have to match pattern - // variables as borrows while executing the guard. This ensures - // the guard can't consume or modify the value without us committing to this - // case branch. If the final pattern match is only borrowing as well, + // If the final pattern match is only borrowing as well, // we can bind the variables immediately here too. - if (hasGuard || *ownership <= ValueOwnership::Shared) { + if (*ownership <= ValueOwnership::Shared) { bindIrrefutableBorrows(clauses[row], args); } if (hasGuard) { - this->emitGuardBranch(guardExpr, guardExpr, failure); + // The guard will bind borrows locally if necessary. + this->emitGuardBranch(guardExpr, guardExpr, failure, + clauses[row], args); } if (*ownership > ValueOwnership::Shared) { - // Unbind the variables and unborrow the subject so we can consume it - // later. unbindAndEndBorrows(clauses[row], args); } - } else { // Bind the rest of the patterns. // For noncopyable bindings, this will bind them as borrows initially if there @@ -1211,7 +1208,8 @@ void PatternMatchEmission::emitWildcardDispatch(ClauseMatrix &clauses, // Emit the guard branch, if it exists. if (guardExpr) { - this->emitGuardBranch(guardExpr, guardExpr, failure); + this->emitGuardBranch(guardExpr, guardExpr, failure, + clauses[row], args); } } @@ -1244,7 +1242,8 @@ bindRefutablePatterns(const ClauseRow &row, ArgArray args, FullExpr scope(SGF.Cleanups, CleanupLocation(pattern)); bindVariable(pattern, exprPattern->getMatchVar(), args[i], /*isForSuccess*/ false, /* hasMultipleItems */ false); - emitGuardBranch(pattern, exprPattern->getMatchExpr(), failure); + emitGuardBranch(pattern, exprPattern->getMatchExpr(), failure, + row, args); break; } default: @@ -1391,14 +1390,22 @@ void PatternMatchEmission::bindBorrow(Pattern *pattern, VarDecl *var, ConsumableManagedValue value) { assert(value.getFinalConsumption() == CastConsumptionKind::BorrowAlways); - SGF.VarLocs[var] = SILGenFunction::VarLoc::get( - value.asBorrowedOperand2(SGF, pattern).getValue()); + auto bindValue = value.asBorrowedOperand2(SGF, pattern).getFinalManagedValue(); + if (bindValue.getType().isObject()) { + // Create a notional copy for the borrow checker to use. + bindValue = bindValue.copy(SGF, pattern); + } + bindValue = SGF.B.createMarkUnresolvedNonCopyableValueInst(pattern, bindValue, + MarkUnresolvedNonCopyableValueInst::CheckKind::NoConsumeOrAssign); + + SGF.VarLocs[var] = SILGenFunction::VarLoc::get(bindValue.getValue()); } /// Evaluate a guard expression and, if it returns false, branch to /// the given destination. void PatternMatchEmission::emitGuardBranch(SILLocation loc, Expr *guard, - const FailureHandler &failure) { + const FailureHandler &failure, + const ClauseRow &row, ArgArray args){ SILBasicBlock *falseBB = SGF.B.splitBlockForFallthrough(); SILBasicBlock *trueBB = SGF.B.splitBlockForFallthrough(); @@ -1406,6 +1413,15 @@ void PatternMatchEmission::emitGuardBranch(SILLocation loc, Expr *guard, SILValue testBool; { FullExpr scope(SGF.Cleanups, CleanupLocation(guard)); + + // If the final pattern match is destructive, then set up borrow bindings + // to evaluate the guard expression without allowing it to destruct the + // subject yet. + if (auto ownership = getNoncopyableOwnership()) { + if (*ownership > ValueOwnership::Shared) { + bindIrrefutableBorrows(row, args); + } + } testBool = SGF.emitRValueAsSingleValue(guard).getUnmanagedValue(); } @@ -2720,33 +2736,28 @@ void PatternMatchEmission::emitDestructiveCaseBlocks() { CleanupState::PersistentlyActive); } - { - // Create a scope to break down the subject value. - Scope caseScope(SGF, pattern); - - // Clone the original subject's cleanup state so that it will be reliably - // consumed in this scope, while leaving the original for other case - // blocks to re-consume. - ManagedValue subject = SGF.emitManagedRValueWithCleanup( - NoncopyableConsumableValue.forward(SGF)); - - // TODO: handle fallthroughs and multiple cases bindings - // In those cases we'd need to forward bindings through the shared case - // destination blocks. - assert(!stmt->hasFallthroughDest() - && stmt->getCaseLabelItems().size() == 1); - - // Bind variables from the pattern. - if (stmt->hasCaseBodyVariables()) { - ConsumingPatternBindingVisitor(*this, stmt) - .visit(pattern, subject); - } - - // By this point, whatever parts of the value we bound are forwarded into - // variables, so we can pop the scope and destroy the remainder before - // entering the case body (TODO: or common block). - } + // Create a scope to break down the subject value. + Scope caseScope(SGF, pattern); + // Clone the original subject's cleanup state so that it will be reliably + // consumed in this scope, while leaving the original for other case + // blocks to re-consume. + ManagedValue subject = SGF.emitManagedRValueWithCleanup( + NoncopyableConsumableValue.forward(SGF)); + + // TODO: handle fallthroughs and multiple cases bindings + // In those cases we'd need to forward bindings through the shared case + // destination blocks. + assert(!stmt->hasFallthroughDest() + && stmt->getCaseLabelItems().size() == 1); + + // Bind variables from the pattern. + if (stmt->hasCaseBodyVariables()) { + ConsumingPatternBindingVisitor(*this, stmt) + .visit(pattern, subject); + } + + // Emit the case body. emitCaseBody(stmt); } } diff --git a/lib/SILOptimizer/Mandatory/MoveOnlyBorrowToDestructureUtils.cpp b/lib/SILOptimizer/Mandatory/MoveOnlyBorrowToDestructureUtils.cpp index 5bec8d043e4d8..723dab6bb3a4f 100644 --- a/lib/SILOptimizer/Mandatory/MoveOnlyBorrowToDestructureUtils.cpp +++ b/lib/SILOptimizer/Mandatory/MoveOnlyBorrowToDestructureUtils.cpp @@ -372,6 +372,29 @@ bool Implementation::gatherUses(SILValue value) { } case OperandOwnership::GuaranteedForwarding: { + // Always treat switches as full liveness uses of the enum being switched + // since the control flow is significant, and we can't destructure through + // the switch dispatch. If the final pattern match ends up destructuring + // the value, then SILGen emits that as a separate access. + if (auto switchEnum = dyn_cast(nextUse->getUser())) { + auto leafRange = TypeTreeLeafTypeRange::get(switchEnum->getOperand(), + getRootValue()); + if (!leafRange) { + LLVM_DEBUG(llvm::dbgs() << " Failed to compute leaf range?!\n"); + return false; + } + + LLVM_DEBUG(llvm::dbgs() << " Found non lifetime ending use!\n"); + blocksToUses.insert(nextUse->getParentBlock(), + {nextUse, + {liveness.getNumSubElements(), *leafRange, + false /*is lifetime ending*/}}); + liveness.updateForUse(nextUse->getUser(), *leafRange, + false /*is lifetime ending*/); + instToInterestingOperandIndexMap.insert(nextUse->getUser(), nextUse); + continue; + } + // Look through guaranteed forwarding if we have at least one non-trivial // value. If we have all non-trivial values, treat this as a liveness use. SmallVector forwardedValues; @@ -1044,6 +1067,23 @@ static void insertEndBorrowsForNonConsumingUse(Operand *op, borrow); return true; }); + } else if (auto swi = dyn_cast(op->getUser())) { + LLVM_DEBUG(llvm::dbgs() << " -- Ending borrow for switch:\n" + " "; + swi->print(llvm::dbgs())); + // End the borrow where the original borrow of the subject was ended. + // TODO: handle if the switch isn't directly on a borrow? + auto beginBorrow = cast(swi->getOperand()); + BorrowingOperand(&beginBorrow->getOperandRef()) + .visitScopeEndingUses([&](Operand *endScope) -> bool { + auto *endScopeInst = endScope->getUser(); + LLVM_DEBUG(llvm::dbgs() << " "; + endScopeInst->print(llvm::dbgs())); + SILBuilderWithScope endBuilder(endScopeInst); + endBuilder.createEndBorrow(getSafeLoc(endScopeInst), + borrow); + return true; + }); } else { auto *nextInst = op->getUser()->getNextInstruction(); LLVM_DEBUG(llvm::dbgs() << " -- Ending borrow after momentary use at: "; @@ -1521,9 +1561,17 @@ static bool gatherBorrows(SILValue rootValue, static bool gatherSwitchEnum(SILValue value, SmallVectorImpl &switchEnumWorklist) { + auto *fn = value->getFunction(); + auto &C = fn->getASTContext(); + // TODO: For borrowing switches, we want to leave the switch as a borrowing + // operation. This phase of the pass should become moot once borrowing + // switch is on by default. + if (C.LangOpts.hasFeature(Feature::BorrowingSwitch)) { + return true; + } + LLVM_DEBUG(llvm::dbgs() << "Gathering switch enums for value: " << *value); - auto *fn = value->getFunction(); StackList useWorklist(fn); for (auto *use : value->getUses()) { useWorklist.push_back(use); @@ -1639,8 +1687,10 @@ gatherSwitchEnum(SILValue value, //===----------------------------------------------------------------------===// bool BorrowToDestructureTransform::transform() { - LLVM_DEBUG(llvm::dbgs() << "Performing Borrow To Destructure Tranform!\n"); auto *fn = mmci->getFunction(); + LLVM_DEBUG(llvm::dbgs() << "Performing Borrow To Destructure Transform!\n"; + fn->print(llvm::dbgs())); + auto &C = fn->getASTContext(); StackList borrowWorklist(mmci->getFunction()); // If we failed to gather borrows due to the transform not understanding part diff --git a/lib/SILOptimizer/Mandatory/MoveOnlyObjectCheckerUtils.cpp b/lib/SILOptimizer/Mandatory/MoveOnlyObjectCheckerUtils.cpp index cd944dc47881f..5cb384918c184 100644 --- a/lib/SILOptimizer/Mandatory/MoveOnlyObjectCheckerUtils.cpp +++ b/lib/SILOptimizer/Mandatory/MoveOnlyObjectCheckerUtils.cpp @@ -488,10 +488,10 @@ void MoveOnlyObjectCheckerPImpl::check(DominanceInfo *domTree, // Handle: // - // bb0(%0 : @guaranteed $Type): - // %1 = copy_value %0 - // %2 = mark_unresolved_non_copyable_value [no_consume_or_assign] %1 - if (auto *arg = dyn_cast(i->getOperand(0))) { + // bb(%arg : @guaranteed $Type): + // %copy = copy_value %arg + // %mark = mark_unresolved_non_copyable_value [no_consume_or_assign] %copy + if (auto *arg = dyn_cast(i->getOperand(0))) { if (arg->getOwnershipKind() == OwnershipKind::Guaranteed) { for (auto *use : markedInst->getConsumingUses()) { destroys.push_back(cast(use->getUser())); From 067f1d3f1ce0be15852d14682689400e814faf54 Mon Sep 17 00:00:00 2001 From: Joe Groff Date: Wed, 31 Jan 2024 20:46:01 -0800 Subject: [PATCH 2/2] SILGen: Don't mark_unresolved copyable borrows from noncopyable switches --- lib/SILGen/SILGenPattern.cpp | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/lib/SILGen/SILGenPattern.cpp b/lib/SILGen/SILGenPattern.cpp index a54dc77531d0d..2d3b4b1c0dadc 100644 --- a/lib/SILGen/SILGenPattern.cpp +++ b/lib/SILGen/SILGenPattern.cpp @@ -1391,13 +1391,14 @@ void PatternMatchEmission::bindBorrow(Pattern *pattern, VarDecl *var, assert(value.getFinalConsumption() == CastConsumptionKind::BorrowAlways); auto bindValue = value.asBorrowedOperand2(SGF, pattern).getFinalManagedValue(); - if (bindValue.getType().isObject()) { - // Create a notional copy for the borrow checker to use. - bindValue = bindValue.copy(SGF, pattern); + if (bindValue.getType().isMoveOnly()) { + if (bindValue.getType().isObject()) { + // Create a notional copy for the borrow checker to use. + bindValue = bindValue.copy(SGF, pattern); + } + bindValue = SGF.B.createMarkUnresolvedNonCopyableValueInst(pattern, bindValue, + MarkUnresolvedNonCopyableValueInst::CheckKind::NoConsumeOrAssign); } - bindValue = SGF.B.createMarkUnresolvedNonCopyableValueInst(pattern, bindValue, - MarkUnresolvedNonCopyableValueInst::CheckKind::NoConsumeOrAssign); - SGF.VarLocs[var] = SILGenFunction::VarLoc::get(bindValue.getValue()); }