From ebcab8f2eeb4a5e03349a36d9ee0b7e5251da3dd Mon Sep 17 00:00:00 2001 From: Michael Gottesman Date: Tue, 12 Nov 2024 16:21:53 -0800 Subject: [PATCH 1/5] [silgen] Change ArgEmitter to use an OptionSet before I add another option. NFCI. --- lib/SILGen/SILGenApply.cpp | 56 ++++++++++++++++++++------------------ 1 file changed, 30 insertions(+), 26 deletions(-) diff --git a/lib/SILGen/SILGenApply.cpp b/lib/SILGen/SILGenApply.cpp index 66d95c7f78ec2..86182d2e2921a 100644 --- a/lib/SILGen/SILGenApply.cpp +++ b/lib/SILGen/SILGenApply.cpp @@ -3412,11 +3412,18 @@ Expr *ArgumentSource::findStorageReferenceExprForBorrow() && { namespace { class ArgEmitter { +public: + enum Flag { + IsYield = 0x1, + IsForCoroutine = 0x2, + }; + using OptionSet = OptionSet; + +private: SILGenFunction &SGF; SILLocation ApplyLoc; SILFunctionTypeRepresentation Rep; - bool IsYield; - bool IsForCoroutine; + OptionSet Options; ForeignInfo Foreign; ClaimedParamsRef ParamInfos; SmallVectorImpl &Args; @@ -3427,13 +3434,11 @@ class ArgEmitter { public: ArgEmitter(SILGenFunction &SGF, SILLocation applyLoc, - SILFunctionTypeRepresentation Rep, - bool isYield, bool isForCoroutine, ClaimedParamsRef paramInfos, - SmallVectorImpl &args, + SILFunctionTypeRepresentation Rep, OptionSet options, + ClaimedParamsRef paramInfos, SmallVectorImpl &args, SmallVectorImpl &delayedArgs, const ForeignInfo &foreign) - : SGF(SGF), ApplyLoc(applyLoc), - Rep(Rep), IsYield(isYield), IsForCoroutine(isForCoroutine), + : SGF(SGF), ApplyLoc(applyLoc), Rep(Rep), Options(options), Foreign(foreign), ParamInfos(paramInfos), Args(args), DelayedArguments(delayedArgs) {} @@ -3559,7 +3564,7 @@ class ArgEmitter { // If we have a guaranteed +0 parameter... if (param.isGuaranteedInCaller() || isShared) { // And this is a yield, emit a borrowed r-value. - if (IsYield) { + if (Options.contains(Flag::IsYield)) { if (tryEmitBorrowed(std::move(arg), loweredSubstArgType, loweredSubstParamType, origParamType, paramSlice)) return; @@ -3610,7 +3615,7 @@ class ArgEmitter { // Handle yields of storage reference expressions specially so that we // don't emit them as +1 r-values and then expand. - if (IsYield) { + if (Options.contains(Flag::IsYield)) { if (auto result = std::move(arg).findStorageReferenceExprForBorrow()) { emitExpandedBorrowed(result, origParamType); return; @@ -3858,7 +3863,8 @@ class ArgEmitter { auto convertOwnershipConvention = [&](ManagedValue value) { return convertOwnershipConventionGivenParamInfo( - SGF, param, origParam, value, loc, IsForCoroutine); + SGF, param, origParam, value, loc, + Options.contains(Flag::IsForCoroutine)); }; auto contexts = getRValueEmissionContexts(loweredSubstArgType, param); @@ -4423,10 +4429,9 @@ void DelayedArgument::emitDefaultArgument(SILGenFunction &SGF, SmallVector loweredArgs; SmallVector delayedArgs; - auto emitter = ArgEmitter(SGF, info.loc, info.functionRepresentation, - /*yield*/ false, /*coroutine*/ false, - info.paramsToEmit, loweredArgs, - delayedArgs, ForeignInfo{}); + auto emitter = + ArgEmitter(SGF, info.loc, info.functionRepresentation, {}, + info.paramsToEmit, loweredArgs, delayedArgs, ForeignInfo{}); emitter.emitSingleArg(ArgumentSource(info.loc, std::move(value)), info.origResultType); @@ -4818,9 +4823,11 @@ class CallSite { const ForeignInfo &foreign) && { auto params = lowering.claimParams(origFormalType, getParams(), foreign); - ArgEmitter emitter(SGF, Loc, lowering.Rep, /*yield*/ false, - /*isForCoroutine*/ substFnType->isCoroutine(), params, - args, delayedArgs, foreign); + ArgEmitter::OptionSet options; + if (substFnType->isCoroutine()) + options |= ArgEmitter::IsForCoroutine; + ArgEmitter emitter(SGF, Loc, lowering.Rep, options, params, args, + delayedArgs, foreign); emitter.emitPreparedArgs(std::move(Args), origFormalType); } @@ -6069,8 +6076,8 @@ void SILGenFunction::emitYield(SILLocation loc, origYield.getConvention()}); } - ArgEmitter emitter(*this, loc, fnType->getRepresentation(), /*yield*/ true, - /*isForCoroutine*/ false, ClaimedParamsRef(substYieldTys), + ArgEmitter emitter(*this, loc, fnType->getRepresentation(), + ArgEmitter::IsYield, ClaimedParamsRef(substYieldTys), yieldArgs, delayedArgs, ForeignInfo{}); for (auto i : indices(valueSources)) { @@ -7077,10 +7084,9 @@ static void emitPseudoFunctionArguments(SILGenFunction &SGF, SmallVector argValues; SmallVector delayedArgs; - ArgEmitter emitter(SGF, applyLoc, SILFunctionTypeRepresentation::Thin, - /*yield*/ false, - /*isForCoroutine*/ false, ClaimedParamsRef(substParamTys), - argValues, delayedArgs, ForeignInfo{}); + ArgEmitter emitter(SGF, applyLoc, SILFunctionTypeRepresentation::Thin, {}, + ClaimedParamsRef(substParamTys), argValues, delayedArgs, + ForeignInfo{}); emitter.emitPreparedArgs(std::move(args), origFnType); @@ -7765,9 +7771,7 @@ SmallVector SILGenFunction::emitKeyPathSubscriptOperands( SmallVector argValues; SmallVector delayedArgs; - ArgEmitter emitter(*this, loc, fnType->getRepresentation(), - /*yield*/ false, - /*isForCoroutine*/ false, + ArgEmitter emitter(*this, loc, fnType->getRepresentation(), {}, ClaimedParamsRef(fnType->getParameters()), argValues, delayedArgs, ForeignInfo{}); From d541190a5a344e5f35d92515218ad808a39d6d33 Mon Sep 17 00:00:00 2001 From: Michael Gottesman Date: Fri, 15 Nov 2024 15:35:35 -0800 Subject: [PATCH 2/5] [silgen] Refactor out how we compute the actor isolation for a SILFunction so we can reuse it in other contexts. I also want to extend it and did not want to have to copy/paste this code into multiple places. The small test tweak occurs since I changed the initializer SILGen emission code to set the declref field of SILFunctions to the actual decl ref which we did not before. So we got a more specific diagnostic. --- lib/SILGen/SILGen.cpp | 45 ++++++++++--------- test/Concurrency/transfernonsendable.swift | 2 +- .../transfernonsendable_typed_errors.swift | 2 +- 3 files changed, 26 insertions(+), 23 deletions(-) diff --git a/lib/SILGen/SILGen.cpp b/lib/SILGen/SILGen.cpp index 9976a2a2eebbe..51e787fefebe7 100644 --- a/lib/SILGen/SILGen.cpp +++ b/lib/SILGen/SILGen.cpp @@ -731,6 +731,25 @@ static bool isEmittedOnDemand(SILModule &M, SILDeclRef constant) { return false; } +static ActorIsolation getActorIsolationForFunction(SILFunction &fn) { + if (auto constant = fn.getDeclRef()) { + if (constant.kind == SILDeclRef::Kind::Deallocator) { + // Deallocating destructor is always nonisolated. Isolation of the deinit + // applies only to isolated deallocator and destroyer. + return ActorIsolation::forNonisolated(false); + } + + // If we have actor isolation for our constant, put the isolation onto the + // function. If the isolation is unspecified, we do not return it. + if (auto isolation = + getActorIsolationOfContext(constant.getInnermostDeclContext())) + return isolation; + } + + // Otherwise, return for unspecified. + return ActorIsolation::forUnspecified(); +} + SILFunction *SILGenModule::getFunction(SILDeclRef constant, ForDefinition_t forDefinition) { // If we already emitted the function, return it. @@ -756,16 +775,8 @@ SILFunction *SILGenModule::getFunction(SILDeclRef constant, return IGM.getFunction(constant, NotForDefinition); }); - if (constant.kind == SILDeclRef::Kind::Deallocator) { - // Deallocating destructor is always nonisolated. - // Isolation of the deinit applies only to isolated deallocator and - // destroyer. - F->setActorIsolation(ActorIsolation::forNonisolated(false)); - } else { - // If we have global actor isolation for our constant, put the isolation onto - // the function. - F->setActorIsolation(getActorIsolationOfContext(constant.getInnermostDeclContext())); - } + F->setDeclRef(constant); + F->setActorIsolation(getActorIsolationForFunction(*F)); assert(F && "SILFunction should have been defined"); @@ -1254,23 +1265,15 @@ void SILGenModule::preEmitFunction(SILDeclRef constant, SILFunction *F, F->setGenericEnvironment(genericEnv, capturedEnvs, forwardingSubs); } - if (constant.kind == SILDeclRef::Kind::Deallocator) { - // Deallocating destructor is always nonisolated. - // Isolation of the deinit applies only to isolated deallocator and - // destroyer. - F->setActorIsolation(ActorIsolation::forNonisolated(false)); - } else { - // If we have global actor isolation for our constant, put the isolation - // onto the function. - F->setActorIsolation(getActorIsolationOfContext(constant.getInnermostDeclContext())); - } - // Create a debug scope for the function using astNode as source location. F->setDebugScope(new (M) SILDebugScope(Loc, F)); // Initialize F with the constant we created for it. F->setDeclRef(constant); + // Set our actor isolation. + F->setActorIsolation(getActorIsolationForFunction(*F)); + LLVM_DEBUG(llvm::dbgs() << "lowering "; F->printName(llvm::dbgs()); llvm::dbgs() << " : "; diff --git a/test/Concurrency/transfernonsendable.swift b/test/Concurrency/transfernonsendable.swift index 969c66d57c43e..d647b1bed4e8d 100644 --- a/test/Concurrency/transfernonsendable.swift +++ b/test/Concurrency/transfernonsendable.swift @@ -1763,7 +1763,7 @@ extension MyActor { _ = sc Task { // expected-tns-warning {{sending value of non-Sendable type '() async -> ()' risks causing data races}} - // expected-tns-note @-1 {{Passing value of non-Sendable type '() async -> ()' as a 'sending' argument risks causing races in between local and caller code}} + // expected-tns-note @-1 {{Passing value of non-Sendable type '() async -> ()' as a 'sending' argument to initializer 'init(priority:operation:)' risks causing races in between local and caller code}} _ = sc } diff --git a/test/Concurrency/transfernonsendable_typed_errors.swift b/test/Concurrency/transfernonsendable_typed_errors.swift index d0e874ca882f8..72d45bac13cb5 100644 --- a/test/Concurrency/transfernonsendable_typed_errors.swift +++ b/test/Concurrency/transfernonsendable_typed_errors.swift @@ -60,7 +60,7 @@ extension MyActor { _ = sc Task { // expected-error {{sending value of non-Sendable type '() async -> ()' risks causing data races}} - // expected-note @-1 {{Passing value of non-Sendable type '() async -> ()' as a 'sending' argument risks causing races in between local and caller code}} + // expected-note @-1 {{Passing value of non-Sendable type '() async -> ()' as a 'sending' argument to initializer 'init(priority:operation:)' risks causing races in between local and caller code}} _ = sc } From afaf393fd45cd0585de2ada16cca1d63d2e33baa Mon Sep 17 00:00:00 2001 From: Michael Gottesman Date: Mon, 18 Nov 2024 11:07:18 -0800 Subject: [PATCH 3/5] [ast] Add a helper method to SILFunctionType to grab the SILParameterInfo of the type's isolated parameter if one exists. --- include/swift/AST/Types.h | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/include/swift/AST/Types.h b/include/swift/AST/Types.h index 1de4cf8b482ad..b48aa39e5fc47 100644 --- a/include/swift/AST/Types.h +++ b/include/swift/AST/Types.h @@ -5348,6 +5348,18 @@ class SILFunctionType final return getParameters().back(); } + /// Return SILParameterInfo for the isolated parameter in this SILFunctionType + /// if one exists. Returns None otherwise. + std::optional maybeGetIsolatedParameter() const { + for (auto param : getParameters()) { + if (param.hasOption(SILParameterInfo::Isolated)) { + return param; + } + } + + return {}; + } + struct IndirectMutatingParameterFilter { bool operator()(SILParameterInfo param) const { return param.isIndirectMutating(); From d33f8190380b4877b8baf457a4d31f1cb796bb10 Mon Sep 17 00:00:00 2001 From: Michael Gottesman Date: Mon, 18 Nov 2024 13:57:36 -0800 Subject: [PATCH 4/5] [region-isolation] Move freeform logging on the specific error we are emitting into a method on the error itself. I am doing this since I discovered that we are not printing certain errors as early as we used to (due to the refactoring I did here), which makes it harder to see the errors that we are emitting while processing individual instructions and before we run the actual dataflow. A nice side-effect of this is that it will make it easy to dump the error in the debugger rather than having to wait until the point in the code where the normal logging takes place. --- .../swift/SILOptimizer/Utils/PartitionUtils.h | 52 ++++++++++++ .../SILOptimizer/Utils/SILIsolationInfo.h | 1 + .../Mandatory/SendNonSendable.cpp | 69 ++------------- lib/SILOptimizer/Utils/PartitionUtils.cpp | 85 +++++++++++++++++-- 4 files changed, 136 insertions(+), 71 deletions(-) diff --git a/include/swift/SILOptimizer/Utils/PartitionUtils.h b/include/swift/SILOptimizer/Utils/PartitionUtils.h index 0bbf2a2bb9c3b..8e9e37be789d5 100644 --- a/include/swift/SILOptimizer/Utils/PartitionUtils.h +++ b/include/swift/SILOptimizer/Utils/PartitionUtils.h @@ -88,6 +88,7 @@ namespace swift { class Partition; class SendingOperandToStateMap; +class RegionAnalysisValueMap; /// The representative value of the equivalence class that makes up a tracked /// value. @@ -380,6 +381,7 @@ class IsolationHistory::Factory { IsolationHistory get() { return IsolationHistory(this); } }; +/// A struct that represents a specific "sending" operand of an ApplySite. struct SendingOperandState { /// The dynamic isolation info of the region of value when we sent. /// @@ -954,6 +956,12 @@ class PartitionOpError { const PartitionOp *op; UnknownCodePatternError(const PartitionOp &op) : op(&op) {} + + void print(llvm::raw_ostream &os, RegionAnalysisValueMap &valueMap) const; + + SWIFT_DEBUG_DUMPER(dump(RegionAnalysisValueMap &valueMap)) { + print(llvm::dbgs(), valueMap); + } }; struct LocalUseAfterSendError { @@ -964,6 +972,12 @@ class PartitionOpError { LocalUseAfterSendError(const PartitionOp &op, Element elt, Operand *sendingOp) : op(&op), sentElement(elt), sendingOp(sendingOp) {} + + void print(llvm::raw_ostream &os, RegionAnalysisValueMap &valueMap) const; + + SWIFT_DEBUG_DUMPER(dump(RegionAnalysisValueMap &valueMap)) { + print(llvm::dbgs(), valueMap); + } }; struct SentNeverSendableError { @@ -975,6 +989,12 @@ class PartitionOpError { SILDynamicMergedIsolationInfo isolationRegionInfo) : op(&op), sentElement(sentElement), isolationRegionInfo(isolationRegionInfo) {} + + void print(llvm::raw_ostream &os, RegionAnalysisValueMap &valueMap) const; + + SWIFT_DEBUG_DUMPER(dump(RegionAnalysisValueMap &valueMap)) { + print(llvm::dbgs(), valueMap); + } }; struct AssignNeverSendableIntoSendingResultError { @@ -992,6 +1012,12 @@ class PartitionOpError { : op(&op), destElement(destElement), destValue(destValue), srcElement(srcElement), srcValue(srcValue), srcIsolationRegionInfo(srcIsolationRegionInfo) {} + + void print(llvm::raw_ostream &os, RegionAnalysisValueMap &valueMap) const; + + SWIFT_DEBUG_DUMPER(dump(RegionAnalysisValueMap &valueMap)) { + print(llvm::dbgs(), valueMap); + } }; struct InOutSendingNotInitializedAtExitError { @@ -1002,6 +1028,12 @@ class PartitionOpError { InOutSendingNotInitializedAtExitError(const PartitionOp &op, Element elt, Operand *sendingOp) : op(&op), sentElement(elt), sendingOp(sendingOp) {} + + void print(llvm::raw_ostream &os, RegionAnalysisValueMap &valueMap) const; + + SWIFT_DEBUG_DUMPER(dump(RegionAnalysisValueMap &valueMap)) { + print(llvm::dbgs(), valueMap); + } }; struct InOutSendingNotDisconnectedAtExitError { @@ -1013,6 +1045,12 @@ class PartitionOpError { const PartitionOp &op, Element elt, SILDynamicMergedIsolationInfo isolation) : op(&op), inoutSendingElement(elt), isolationInfo(isolation) {} + + void print(llvm::raw_ostream &os, RegionAnalysisValueMap &valueMap) const; + + SWIFT_DEBUG_DUMPER(dump(RegionAnalysisValueMap &valueMap)) { + print(llvm::dbgs(), valueMap); + } }; #define PARTITION_OP_ERROR(NAME) \ @@ -1067,6 +1105,20 @@ class PartitionOpError { Kind getKind() const { return kind; } + void print(llvm::raw_ostream &os, RegionAnalysisValueMap &valueMap) const { + switch (getKind()) { +#define PARTITION_OP_ERROR(NAME) \ + case NAME: \ + return get##NAME##Error().print(os, valueMap); +#include "PartitionOpError.def" + } + llvm_unreachable("Covered switch isn't covered?!"); + } + + SWIFT_DEBUG_DUMPER(dump(RegionAnalysisValueMap &valueMap)) { + return print(llvm::dbgs(), valueMap); + } + #define PARTITION_OP_ERROR(NAME) \ NAME##Error get##NAME##Error() const { \ assert(getKind() == Kind::NAME); \ diff --git a/include/swift/SILOptimizer/Utils/SILIsolationInfo.h b/include/swift/SILOptimizer/Utils/SILIsolationInfo.h index 2dad823e72b2e..114759dcc7b17 100644 --- a/include/swift/SILOptimizer/Utils/SILIsolationInfo.h +++ b/include/swift/SILOptimizer/Utils/SILIsolationInfo.h @@ -507,6 +507,7 @@ class SILDynamicMergedIsolationInfo { operator bool() const { return bool(innerInfo); } SILIsolationInfo *operator->() { return &innerInfo; } + const SILIsolationInfo *operator->() const { return &innerInfo; } SILIsolationInfo getIsolationInfo() const { return innerInfo; } diff --git a/lib/SILOptimizer/Mandatory/SendNonSendable.cpp b/lib/SILOptimizer/Mandatory/SendNonSendable.cpp index 1ea546bfbac15..d92446e285c4c 100644 --- a/lib/SILOptimizer/Mandatory/SendNonSendable.cpp +++ b/lib/SILOptimizer/Mandatory/SendNonSendable.cpp @@ -2335,15 +2335,7 @@ struct DiagnosticEvaluator final } } - auto rep = info->getValueMap().getRepresentative(sentElement); - REGIONBASEDISOLATION_LOG( - llvm::dbgs() << " Emitting Error. Kind: Use After Send\n" - << " Sending Inst: " << *sendingOp->getUser() - << " Sending Op Value: " << sendingOp->get() - << " Require Inst: " << *partitionOp.getSourceInst() - << " ID: %%" << sentElement << "\n" - << " Rep: " << *rep << " Sending Op Num: " - << sendingOp->getOperandNumber() << '\n'); + REGIONBASEDISOLATION_LOG(error.print(llvm::dbgs(), info->getValueMap())); sendingOpToRequireInstMultiMap.insert( sendingOp, RequireInst::forUseAfterSend(partitionOp.getSourceInst())); } @@ -2351,20 +2343,10 @@ struct DiagnosticEvaluator final void handleInOutSendingNotInitializedAtExitError( InOutSendingNotInitializedAtExitError error) const { const PartitionOp &partitionOp = *error.op; - Element inoutSendingVal = error.sentElement; Operand *sendingOp = error.sendingOp; - auto rep = info->getValueMap().getRepresentative(inoutSendingVal); - REGIONBASEDISOLATION_LOG( - llvm::dbgs() - << " Emitting Error. Kind: InOut Not Reinitialized At End Of " - "Function\n" - << " Sending Inst: " << *sendingOp->getUser() - << " Sending Op Value: " << sendingOp->get() - << " Require Inst: " << *partitionOp.getSourceInst() - << " ID: %%" << inoutSendingVal << "\n" - << " Rep: " << *rep - << " Sending Op Num: " << sendingOp->getOperandNumber() << '\n'); + REGIONBASEDISOLATION_LOG(error.print(llvm::dbgs(), info->getValueMap())); + sendingOpToRequireInstMultiMap.insert( sendingOp, RequireInst::forInOutReinitializationNeeded( partitionOp.getSourceInst())); @@ -2496,18 +2478,7 @@ void SendNonSendableImpl::emitVerbatimErrors() { llvm_unreachable("Handled elsewhere"); case PartitionOpError::AssignNeverSendableIntoSendingResult: { auto error = erasedError.getAssignNeverSendableIntoSendingResultError(); - auto srcRep = - info->getValueMap().getRepresentativeValue(error.srcElement); - REGIONBASEDISOLATION_LOG( - llvm::dbgs() - << " Emitting Error. Kind: Assign Isolated Into Sending Result!\n" - << " Assign Inst: " << *error.op->getSourceInst() - << " Dest Value: " << *error.destValue - << " Dest Element: " << error.destElement << '\n' - << " Src Value: " << error.srcValue - << " Src Element: " << error.srcElement << '\n' - << " Src Rep: " << srcRep - << " Src Isolation: " << error.srcIsolationRegionInfo << '\n'); + REGIONBASEDISOLATION_LOG(error.print(llvm::dbgs(), info->getValueMap())); AssignIsolatedIntoSendingResultDiagnosticEmitter emitter( error.op->getSourceOp(), error.destValue, error.srcValue, error.srcIsolationRegionInfo); @@ -2520,16 +2491,7 @@ void SendNonSendableImpl::emitVerbatimErrors() { info->getValueMap().getRepresentative(error.inoutSendingElement); auto isolationRegionInfo = error.isolationInfo; - REGIONBASEDISOLATION_LOG( - llvm::dbgs() - << " Emitting Error. Kind: InOut Sending ActorIsolated " - "at end of " - "Function Error!\n" - << " ID: %%" << error.inoutSendingElement << "\n" - << " Rep: " << *inoutSendingVal - << " Dynamic Isolation Region: "; - isolationRegionInfo.printForOneLineLogging(llvm::dbgs()); - llvm::dbgs() << '\n'); + REGIONBASEDISOLATION_LOG(error.print(llvm::dbgs(), info->getValueMap())); InOutSendingNotDisconnectedDiagnosticEmitter emitter( cast(error.op->getSourceInst()), inoutSendingVal, @@ -2539,26 +2501,7 @@ void SendNonSendableImpl::emitVerbatimErrors() { } case PartitionOpError::SentNeverSendable: { auto e = erasedError.getSentNeverSendableError(); - auto errorLog = [&] { - llvm::dbgs() << " Emitting Error. Kind: Send Non Sendable\n" - << " ID: %%" << e.sentElement << "\n" - << " Rep: " - << *info->getValueMap().getRepresentative(e.sentElement) - << " Dynamic Isolation Region: "; - e.isolationRegionInfo.printForOneLineLogging(llvm::dbgs()); - llvm::dbgs() << '\n'; - if (auto isolatedValue = - e.isolationRegionInfo->maybeGetIsolatedValue()) { - llvm::dbgs() << " Isolated Value: " << isolatedValue; - auto name = inferNameHelper(isolatedValue); - llvm::dbgs() << " Isolated Value Name: " - << (name.has_value() ? name->get() : "none") << '\n'; - } else { - llvm::dbgs() << " Isolated Value: none\n"; - }; - }; - REGIONBASEDISOLATION_LOG(errorLog()); - + REGIONBASEDISOLATION_LOG(e.print(llvm::dbgs(), info->getValueMap())); SentNeverSendableDiagnosticInferrer diagnosticInferrer( info->getValueMap(), e); diagnosticInferrer.run(); diff --git a/lib/SILOptimizer/Utils/PartitionUtils.cpp b/lib/SILOptimizer/Utils/PartitionUtils.cpp index 94bd2961133ee..f5501cdfcde5a 100644 --- a/lib/SILOptimizer/Utils/PartitionUtils.cpp +++ b/lib/SILOptimizer/Utils/PartitionUtils.cpp @@ -12,19 +12,88 @@ #include "swift/SILOptimizer/Utils/PartitionUtils.h" -#include "swift/AST/ASTWalker.h" -#include "swift/AST/Expr.h" -#include "swift/Basic/Assertions.h" -#include "swift/SIL/ApplySite.h" -#include "swift/SIL/InstructionUtils.h" -#include "swift/SIL/PatternMatch.h" -#include "swift/SIL/SILGlobalVariable.h" +// We only use this so we can implement print on our type erased errors. +#include "swift/SILOptimizer/Analysis/RegionAnalysis.h" #include "swift/SILOptimizer/Utils/VariableNameUtils.h" using namespace swift; -using namespace swift::PatternMatch; using namespace swift::PartitionPrimitives; +//===----------------------------------------------------------------------===// +// MARK: PartitionOpError +//===----------------------------------------------------------------------===// + +void PartitionOpError::UnknownCodePatternError::print( + llvm::raw_ostream &os, RegionAnalysisValueMap &valueMap) const { + os << " Emitting Error. Kind: Unknown Code Pattern Error\n" + << " Inst: " << *op->getSourceInst(); +} + +void PartitionOpError::LocalUseAfterSendError::print( + llvm::raw_ostream &os, RegionAnalysisValueMap &valueMap) const { + os << " Emitting Error. Kind: Use After Send\n" + << " Sending Inst: " << *sendingOp->getUser() + << " Sending Op Value: " << sendingOp->get() + << " Require Inst: " << *op->getSourceInst() << " ID: %%" + << sentElement << "\n" + << " Rep: " << valueMap.getRepresentativeValue(sentElement) + << " Sending Op Num: " << sendingOp->getOperandNumber() << '\n'; +} + +void PartitionOpError::SentNeverSendableError::print( + llvm::raw_ostream &os, RegionAnalysisValueMap &info) const { + os << " Emitting Error. Kind: Sent Non Sendable\n" + << " ID: %%" << sentElement << "\n" + << " Rep: " << *info.getRepresentative(sentElement) + << " Dynamic Isolation Region: "; + isolationRegionInfo.printForOneLineLogging(os); + os << '\n'; + if (auto isolatedValue = isolationRegionInfo->maybeGetIsolatedValue()) { + os << " Isolated Value: " << isolatedValue; + auto name = VariableNameInferrer::inferName(isolatedValue); + os << " Isolated Value Name: " + << (name.has_value() ? name->get() : "none") << '\n'; + } else { + os << " Isolated Value: none\n"; + }; +} + +void PartitionOpError::AssignNeverSendableIntoSendingResultError::print( + llvm::raw_ostream &os, RegionAnalysisValueMap &valueMap) const { + os << " Emitting Error. Kind: Assign Isolated Into Sending Result!\n" + << " Assign Inst: " << *op->getSourceInst() + << " Dest Value: " << *destValue + << " Dest Element: " << destElement << '\n' + << " Src Value: " << srcValue + << " Src Element: " << srcElement << '\n' + << " Src Rep: " << valueMap.getRepresentativeValue(srcElement) + << " Src Isolation: " << srcIsolationRegionInfo << '\n'; +} + +void PartitionOpError::InOutSendingNotInitializedAtExitError::print( + llvm::raw_ostream &os, RegionAnalysisValueMap &valueMap) const { + os << " Emitting Error. Kind: InOut Not Reinitialized At End Of " + "Function\n" + << " Sending Inst: " << *sendingOp->getUser() + << " Sending Op Value: " << sendingOp->get() + << " Require Inst: " << *op->getSourceInst() << " ID: %%" + << sentElement << "\n" + << " Rep: " << valueMap.getRepresentativeValue(sentElement) + << " Sending Op Num: " << sendingOp->getOperandNumber() << '\n'; +} + +void PartitionOpError::InOutSendingNotDisconnectedAtExitError::print( + llvm::raw_ostream &os, RegionAnalysisValueMap &valueMap) const { + os << " Emitting Error. Kind: InOut Sending ActorIsolated " + "at end of " + "Function Error!\n" + << " ID: %%" << inoutSendingElement << "\n" + << " Rep: " << valueMap.getRepresentativeValue(inoutSendingElement) + << " Dynamic Isolation Region: "; + isolationInfo.printForOneLineLogging(os); + os << '\n'; +} + //===----------------------------------------------------------------------===// // MARK: PartitionOp //===----------------------------------------------------------------------===// From d94e4c44346e12317d39a7aaed30c9972ba4e4aa Mon Sep 17 00:00:00 2001 From: Michael Gottesman Date: Mon, 18 Nov 2024 14:02:27 -0800 Subject: [PATCH 5/5] [region-isolation] Using the print method from the previous commit, ensure that we dump out SentNeverSendable, InOutSendingNotDisconnectedAtExit, AssignNeverSendableIntoSendingResult earlier when we initially detect them. This just improves the ability to quickly triage bugs in SendNonSendable. It used to be this way, but in the process of doing some refactoring, I moved the logging too late by mistake. --- lib/SILOptimizer/Mandatory/SendNonSendable.cpp | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/lib/SILOptimizer/Mandatory/SendNonSendable.cpp b/lib/SILOptimizer/Mandatory/SendNonSendable.cpp index d92446e285c4c..96a869d786eea 100644 --- a/lib/SILOptimizer/Mandatory/SendNonSendable.cpp +++ b/lib/SILOptimizer/Mandatory/SendNonSendable.cpp @@ -2316,10 +2316,8 @@ struct DiagnosticEvaluator final void handleLocalUseAfterSend(LocalUseAfterSendError error) const { const auto &partitionOp = *error.op; - Element sentElement = error.sentElement; - Operand *sendingOp = error.sendingOp; - auto &operandState = operandToStateMap.get(sendingOp); + auto &operandState = operandToStateMap.get(error.sendingOp); // Ignore this if we have a gep like instruction that is returning a // sendable type and sendingOp was not set with closure // capture. @@ -2337,7 +2335,7 @@ struct DiagnosticEvaluator final REGIONBASEDISOLATION_LOG(error.print(llvm::dbgs(), info->getValueMap())); sendingOpToRequireInstMultiMap.insert( - sendingOp, RequireInst::forUseAfterSend(partitionOp.getSourceInst())); + error.sendingOp, RequireInst::forUseAfterSend(partitionOp.getSourceInst())); } void handleInOutSendingNotInitializedAtExitError( @@ -2372,6 +2370,11 @@ struct DiagnosticEvaluator final case PartitionOpError::InOutSendingNotDisconnectedAtExit: case PartitionOpError::SentNeverSendable: case PartitionOpError::AssignNeverSendableIntoSendingResult: + // We are going to process these later... but dump so we can see that we + // handled an error here. The rest of the explicit handlers will dump as + // appropriate if they want to emit an error here (some will squelch the + // error). + REGIONBASEDISOLATION_LOG(error.print(llvm::dbgs(), info->getValueMap())); foundVerbatimErrors.emplace_back(error); return; case PartitionOpError::InOutSendingNotInitializedAtExit: {