Skip to content

Commit e88ec99

Browse files
committed
[concurrency] Fix a few issues with @execution(caller)/@execution(concurrent).
Specifically: 1. I made it so that thunks from caller -> concurrent properly ignore the isolated parameter of the thunk when calling the concurrent function. rdar://148112362 2. I made it so that thunks from concurrent -> caller properly create a Optional<any Actor>.none and pass that into the caller function. rdar://148112384 3. I made it so that in cases where we are assigning an @sendable caller to a non-sendable caller variable, we allow for the conversion as long as the parameters/results are sendable as well. rdar://148112532 4. I made it so that when we generate a thunk from @execution(caller) -> @GlobalActor, we mangle in @GlobalActor into the thunk. rdar://148112569 5. I discovered that due to the way we handle function conversion expr/decl ref expr, we were emitted two thunks when we assigned a global @caller function to a local @caller variable. The result is that we would first cast from @caller -> @Concurrent and then back to @caller. The result of this would be that the @caller function would always be called on the global queue. rdar://148112646 I also added a bunch of basic tests as well that showed that this behavior was broken.
1 parent 25c0aa3 commit e88ec99

File tree

7 files changed

+607
-33
lines changed

7 files changed

+607
-33
lines changed

include/swift/AST/Types.h

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3800,6 +3800,12 @@ class AnyFunctionType : public TypeBase {
38003800
/// Return the function type without the throwing.
38013801
AnyFunctionType *getWithoutThrowing() const;
38023802

3803+
/// Return the function type with the given \p isolation.
3804+
AnyFunctionType *getWithIsolation(FunctionTypeIsolation isolation) const;
3805+
3806+
/// Return the function type setting sendable to \p newValue.
3807+
AnyFunctionType *getWithSendable(bool newValue) const;
3808+
38033809
/// True if the parameter declaration it is attached to is guaranteed
38043810
/// to not persist the closure for longer than the duration of the call.
38053811
bool isNoEscape() const {

lib/AST/Type.cpp

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4486,6 +4486,17 @@ AnyFunctionType *AnyFunctionType::getWithoutThrowing() const {
44864486
return withExtInfo(info);
44874487
}
44884488

4489+
AnyFunctionType *
4490+
AnyFunctionType::getWithIsolation(FunctionTypeIsolation isolation) const {
4491+
auto info = getExtInfo().intoBuilder().withIsolation(isolation).build();
4492+
return withExtInfo(info);
4493+
}
4494+
4495+
AnyFunctionType *AnyFunctionType::getWithSendable(bool newValue) const {
4496+
auto info = getExtInfo().intoBuilder().withSendable(newValue).build();
4497+
return withExtInfo(info);
4498+
}
4499+
44894500
std::optional<Type> AnyFunctionType::getEffectiveThrownErrorType() const {
44904501
// A non-throwing function... has no thrown interface type.
44914502
if (!isThrowing())

lib/SILGen/SILGenExpr.cpp

Lines changed: 69 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -487,6 +487,11 @@ namespace {
487487
}
488488
RValue visitFunctionConversionExpr(FunctionConversionExpr *E,
489489
SGFContext C);
490+
491+
/// Helper method for handling function conversion expr to
492+
/// @execution(caller). Returns an empty RValue on failure.
493+
RValue emitExecutionCallerFunctionConversionExpr(FunctionConversionExpr *E,
494+
SGFContext C);
490495
RValue visitActorIsolationErasureExpr(ActorIsolationErasureExpr *E,
491496
SGFContext C);
492497
RValue visitExtractFunctionIsolationExpr(ExtractFunctionIsolationExpr *E,
@@ -1942,9 +1947,58 @@ static ManagedValue convertFunctionRepresentation(SILGenFunction &SGF,
19421947
llvm_unreachable("bad representation");
19431948
}
19441949

1950+
RValue RValueEmitter::emitExecutionCallerFunctionConversionExpr(
1951+
FunctionConversionExpr *e, SGFContext C) {
1952+
CanAnyFunctionType srcType =
1953+
cast<FunctionType>(e->getSubExpr()->getType()->getCanonicalType());
1954+
CanAnyFunctionType destType =
1955+
cast<FunctionType>(e->getType()->getCanonicalType());
1956+
assert(destType->getIsolation().isNonIsolatedCaller() &&
1957+
"Should only call this if destType is non isolated caller");
1958+
1959+
auto *subExpr = e->getSubExpr();
1960+
1961+
// Check if we have a subExpr and if this conversion is just stripping off
1962+
// @Sendable. In such a case, we can look through.
1963+
if (auto *fce = dyn_cast<FunctionConversionExpr>(subExpr)) {
1964+
if (srcType->hasExtInfo() && srcType->getExtInfo().isSendable() &&
1965+
destType->hasExtInfo() && !destType->getExtInfo().isSendable() &&
1966+
destType->getWithSendable(true) == srcType) {
1967+
subExpr = fce->getSubExpr();
1968+
1969+
// We changed our subExpr... so recompute our srcType.
1970+
srcType = cast<FunctionType>(subExpr->getType()->getCanonicalType());
1971+
}
1972+
}
1973+
1974+
// Check if the only difference in between our destType and srcType is our
1975+
// isolation.
1976+
if (!srcType->hasExtInfo() || !destType->hasExtInfo() ||
1977+
destType->getWithIsolation(srcType->getIsolation()) != srcType) {
1978+
return RValue();
1979+
}
1980+
1981+
// Ok, we know that our underlying types are the same. Lets try to emit.
1982+
auto *declRef = dyn_cast<DeclRefExpr>(subExpr);
1983+
if (!declRef)
1984+
return RValue();
1985+
1986+
auto *decl = dyn_cast<FuncDecl>(declRef->getDecl());
1987+
if (!decl || !getActorIsolation(decl).isCallerIsolationInheriting())
1988+
return RValue();
1989+
1990+
// Ok, we found our target.
1991+
SILDeclRef silDeclRef(decl);
1992+
assert(silDeclRef.getParameterListCount() == 1);
1993+
auto substType = cast<AnyFunctionType>(destType);
1994+
auto typeContext = SGF.getFunctionTypeInfo(substType);
1995+
ManagedValue result = SGF.emitClosureValue(
1996+
e, silDeclRef, typeContext, declRef->getDeclRef().getSubstitutions());
1997+
return RValue(SGF, e, destType, result);
1998+
}
1999+
19452000
RValue RValueEmitter::visitFunctionConversionExpr(FunctionConversionExpr *e,
1946-
SGFContext C)
1947-
{
2001+
SGFContext C) {
19482002
CanAnyFunctionType srcType =
19492003
cast<FunctionType>(e->getSubExpr()->getType()->getCanonicalType());
19502004
CanAnyFunctionType destType =
@@ -2041,6 +2095,19 @@ RValue RValueEmitter::visitFunctionConversionExpr(FunctionConversionExpr *e,
20412095
}
20422096
}
20432097

2098+
// Check if we are converting a function to an @execution(caller) from a
2099+
// declref that is also @execution(caller). In such a case, this was a case
2100+
// that was put in by Sema. We do not need a thunk, but just need to recognize
2101+
// this case and elide the conversion. The reason why we need to do this is
2102+
// that otherwise, we put in extra thunks that convert @execution(caller) to
2103+
// @execution(concurrent) back to @execution(caller). This is done b/c we do
2104+
// not represent @execution(caller) in interface types, so the actual decl ref
2105+
// will be viewed as @async () -> ().
2106+
if (destType->getIsolation().isNonIsolatedCaller()) {
2107+
if (RValue rv = emitExecutionCallerFunctionConversionExpr(e, C))
2108+
return rv;
2109+
}
2110+
20442111
// Break the conversion into three stages:
20452112
// 1) changing the representation from foreign to native
20462113
// 2) changing the signature within the representation

lib/SILGen/SILGenFunction.h

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2601,11 +2601,19 @@ class LLVM_LIBRARY_VISIBILITY SILGenFunction
26012601
SILType loweredResultTy,
26022602
SGFContext ctx = SGFContext());
26032603

2604+
enum class ThunkGenFlag {
2605+
None,
2606+
ConvertingToNonIsolatedCaller,
2607+
ConvertingFromNonIsolatedCaller,
2608+
};
2609+
using ThunkGenOptions = OptionSet<ThunkGenFlag>;
2610+
26042611
/// Used for emitting SILArguments of bare functions, such as thunks.
26052612
void collectThunkParams(
26062613
SILLocation loc, SmallVectorImpl<ManagedValue> &params,
26072614
SmallVectorImpl<ManagedValue> *indirectResultParams = nullptr,
2608-
SmallVectorImpl<ManagedValue> *indirectErrorParams = nullptr);
2615+
SmallVectorImpl<ManagedValue> *indirectErrorParams = nullptr,
2616+
ThunkGenOptions options = {});
26092617

26102618
/// Build the type of a function transformation thunk.
26112619
CanSILFunctionType buildThunkType(CanSILFunctionType &sourceType,

lib/SILGen/SILGenPoly.cpp

Lines changed: 112 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -985,7 +985,7 @@ ManagedValue Transform::transformTuple(ManagedValue inputTuple,
985985
void SILGenFunction::collectThunkParams(
986986
SILLocation loc, SmallVectorImpl<ManagedValue> &params,
987987
SmallVectorImpl<ManagedValue> *indirectResults,
988-
SmallVectorImpl<ManagedValue> *indirectErrors) {
988+
SmallVectorImpl<ManagedValue> *indirectErrors, ThunkGenOptions options) {
989989
// Add the indirect results.
990990
for (auto resultTy : F.getConventions().getIndirectSILResultTypes(
991991
getTypeExpansionContext())) {
@@ -1021,7 +1021,16 @@ void SILGenFunction::collectThunkParams(
10211021
// be lowered to their underlying type if allowed by resilience.
10221022
auto inContextParamTy = F.getLoweredType(paramTy.getASTType())
10231023
.getCategoryType(paramTy.getCategory());
1024-
params.push_back(B.createInputFunctionArgument(inContextParamTy, loc));
1024+
auto functionArgument =
1025+
B.createInputFunctionArgument(inContextParamTy, loc);
1026+
1027+
// If we are told to not propagate the isolated parameter and this is the
1028+
// implicit leading/isolated parameter, continue.
1029+
if (options.contains(ThunkGenFlag::ConvertingToNonIsolatedCaller) &&
1030+
param.hasOption(SILParameterInfo::ImplicitLeading) &&
1031+
param.hasOption(SILParameterInfo::Isolated))
1032+
continue;
1033+
params.push_back(functionArgument);
10251034
}
10261035
}
10271036

@@ -1454,15 +1463,18 @@ class TranslateArguments : public ExpanderBase<TranslateArguments, ParamInfo> {
14541463
CanSILFunctionType InnerTypesFuncTy;
14551464
ArrayRef<SILParameterInfo> InnerTypes;
14561465
InnerPackArgGenerator InnerPacks;
1466+
SILGenFunction::ThunkGenOptions Options;
1467+
14571468
public:
14581469
TranslateArguments(SILGenFunction &SGF, SILLocation loc,
14591470
ArrayRef<ManagedValue> outerArgs,
14601471
SmallVectorImpl<ManagedValue> &innerArgs,
14611472
CanSILFunctionType innerTypesFuncTy,
1462-
ArrayRef<SILParameterInfo> innerTypes)
1463-
: ExpanderBase(SGF, loc, outerArgs), InnerArgs(innerArgs),
1464-
InnerTypesFuncTy(innerTypesFuncTy), InnerTypes(innerTypes),
1465-
InnerPacks(*this) {}
1473+
ArrayRef<SILParameterInfo> innerTypes,
1474+
SILGenFunction::ThunkGenOptions options = {})
1475+
: ExpanderBase(SGF, loc, outerArgs), InnerArgs(innerArgs),
1476+
InnerTypesFuncTy(innerTypesFuncTy), InnerTypes(innerTypes),
1477+
InnerPacks(*this), Options(options) {}
14661478

14671479
void process(AbstractionPattern innerOrigFunctionType,
14681480
AnyFunctionType::CanParamArrayRef innerSubstTypes,
@@ -2905,12 +2917,18 @@ static ManagedValue applyTrivialConversions(SILGenFunction &SGF,
29052917
}
29062918

29072919
/// Forward arguments according to a function type's ownership conventions.
2908-
static void forwardFunctionArguments(SILGenFunction &SGF,
2909-
SILLocation loc,
2910-
CanSILFunctionType fTy,
2911-
ArrayRef<ManagedValue> managedArgs,
2912-
SmallVectorImpl<SILValue> &forwardedArgs) {
2920+
static void
2921+
forwardFunctionArguments(SILGenFunction &SGF, SILLocation loc,
2922+
CanSILFunctionType fTy,
2923+
ArrayRef<ManagedValue> managedArgs,
2924+
SmallVectorImpl<SILValue> &forwardedArgs,
2925+
SILGenFunction::ThunkGenOptions options = {}) {
29132926
auto argTypes = fTy->getParameters();
2927+
if (options.contains(
2928+
SILGenFunction::ThunkGenFlag::ConvertingFromNonIsolatedCaller)) {
2929+
argTypes = argTypes.drop_front();
2930+
}
2931+
29142932
for (auto index : indices(managedArgs)) {
29152933
auto arg = managedArgs[index];
29162934
auto argTy = argTypes[index];
@@ -5424,9 +5442,31 @@ static void buildThunkBody(SILGenFunction &SGF, SILLocation loc,
54245442

54255443
FullExpr scope(SGF.Cleanups, CleanupLocation(loc));
54265444

5445+
using ThunkGenFlag = SILGenFunction::ThunkGenFlag;
5446+
auto options = SILGenFunction::ThunkGenOptions();
5447+
5448+
// If our original function was nonisolated caller and our eventual type is
5449+
// just nonisolated, pop the isolated argument.
5450+
//
5451+
// NOTE: We do this early before thunk params so that we avoid pushing the
5452+
// isolated parameter preventing us from having to memcpy over the array.
5453+
if (outputSubstType->isAsync()) {
5454+
if (outputSubstType->getIsolation().getKind() ==
5455+
FunctionTypeIsolation::Kind::NonIsolatedCaller &&
5456+
inputSubstType->getIsolation().getKind() !=
5457+
FunctionTypeIsolation::Kind::NonIsolatedCaller)
5458+
options |= ThunkGenFlag::ConvertingToNonIsolatedCaller;
5459+
5460+
if (outputSubstType->getIsolation().getKind() !=
5461+
FunctionTypeIsolation::Kind::NonIsolatedCaller &&
5462+
inputSubstType->getIsolation().getKind() ==
5463+
FunctionTypeIsolation::Kind::NonIsolatedCaller)
5464+
options |= ThunkGenFlag::ConvertingFromNonIsolatedCaller;
5465+
}
5466+
54275467
SmallVector<ManagedValue, 8> params;
54285468
SmallVector<ManagedValue, 4> indirectResultParams;
5429-
SGF.collectThunkParams(loc, params, &indirectResultParams);
5469+
SGF.collectThunkParams(loc, params, &indirectResultParams, nullptr, options);
54305470

54315471
// Ignore the self parameter at the SIL level. IRGen will use it to
54325472
// recover type metadata.
@@ -5444,6 +5484,16 @@ static void buildThunkBody(SILGenFunction &SGF, SILLocation loc,
54445484
outputErasedIsolation = params.pop_back_val();
54455485
}
54465486

5487+
if (!SGF.F.maybeGetIsolatedArgument() && argTypes.size() &&
5488+
argTypes.front().hasOption(SILParameterInfo::Isolated) &&
5489+
argTypes.front().hasOption(SILParameterInfo::ImplicitLeading))
5490+
options |= ThunkGenFlag::ConvertingFromNonIsolatedCaller;
5491+
5492+
// If we are converting to nonisolated caller, we are going to have an extra
5493+
// parameter in our argTypes that we need to drop.
5494+
if (options.contains(ThunkGenFlag::ConvertingFromNonIsolatedCaller))
5495+
argTypes = argTypes.drop_front();
5496+
54475497
// We may need to establish the right executor for the input function.
54485498
// If both function types are synchronous, whoever calls this thunk is
54495499
// responsible for establishing the executor properly, so we don't need
@@ -5514,11 +5564,9 @@ static void buildThunkBody(SILGenFunction &SGF, SILLocation loc,
55145564
// other direction (the thunk receives an Int like a T, and passes it
55155565
// like a normal Int when calling the inner function).
55165566
SmallVector<ManagedValue, 8> args;
5517-
TranslateArguments(SGF, loc, params, args, fnType, argTypes)
5518-
.process(inputOrigType,
5519-
inputSubstType.getParams(),
5520-
outputOrigType,
5521-
outputSubstType.getParams());
5567+
TranslateArguments(SGF, loc, params, args, fnType, argTypes, options)
5568+
.process(inputOrigType, inputSubstType.getParams(), outputOrigType,
5569+
outputSubstType.getParams());
55225570

55235571
SmallVector<SILValue, 8> argValues;
55245572

@@ -5546,11 +5594,35 @@ static void buildThunkBody(SILGenFunction &SGF, SILLocation loc,
55465594
/*mandatory*/false);
55475595
}
55485596

5597+
// If we are thunking a nonisolated caller to nonisolated or global actor, we
5598+
// need to load the actor.
5599+
if (options.contains(ThunkGenFlag::ConvertingFromNonIsolatedCaller)) {
5600+
auto outputIsolation = outputSubstType->getIsolation();
5601+
switch (outputIsolation.getKind()) {
5602+
case FunctionTypeIsolation::Kind::NonIsolated:
5603+
argValues.push_back(SGF.emitNonIsolatedIsolation(loc).getValue());
5604+
break;
5605+
case FunctionTypeIsolation::Kind::GlobalActor: {
5606+
auto globalActor =
5607+
outputIsolation.getGlobalActorType()->getCanonicalType();
5608+
argValues.push_back(
5609+
SGF.emitGlobalActorIsolation(loc, globalActor).getValue());
5610+
break;
5611+
}
5612+
case FunctionTypeIsolation::Kind::Parameter:
5613+
case FunctionTypeIsolation::Kind::Erased:
5614+
case FunctionTypeIsolation::Kind::NonIsolatedCaller:
5615+
llvm_unreachable("Should never see this");
5616+
break;
5617+
}
5618+
}
5619+
55495620
// Add the rest of the arguments.
5550-
forwardFunctionArguments(SGF, loc, fnType, args, argValues);
5621+
forwardFunctionArguments(SGF, loc, fnType, args, argValues, options);
55515622

55525623
auto fun = fnType->isCalleeGuaranteed() ? fnValue.borrow(SGF, loc).getValue()
55535624
: fnValue.forward(SGF);
5625+
55545626
SILValue innerResult =
55555627
SGF.emitApplyWithRethrow(loc, fun,
55565628
/*substFnType*/ fnValue.getType(),
@@ -5683,18 +5755,29 @@ static ManagedValue createThunk(SILGenFunction &SGF,
56835755
auto toType = expectedType->getWithExtInfo(
56845756
expectedType->getExtInfo().withNoEscape(false));
56855757
CanType dynamicSelfType;
5686-
auto thunkType = SGF.buildThunkType(sourceType, toType,
5687-
inputSubstType,
5688-
outputSubstType,
5689-
genericEnv,
5690-
interfaceSubs,
5691-
dynamicSelfType);
5692-
// An actor-isolated non-async function can be converted to an async function
5693-
// by inserting a hop to the global actor.
5758+
auto thunkType =
5759+
SGF.buildThunkType(sourceType, toType, inputSubstType, outputSubstType,
5760+
genericEnv, interfaceSubs, dynamicSelfType);
56945761
CanType globalActorForThunk;
5695-
if (outputSubstType->isAsync()
5696-
&& !inputSubstType->isAsync()) {
5697-
globalActorForThunk = CanType(inputSubstType->getGlobalActor());
5762+
// If our output type is async...
5763+
if (outputSubstType->isAsync()) {
5764+
// And our input type is not async, we can convert the actor-isolated
5765+
// non-async function to an async function by inserting a hop to the global
5766+
// actor.
5767+
if (!inputSubstType->isAsync()) {
5768+
globalActorForThunk = CanType(inputSubstType->getGlobalActor());
5769+
} else {
5770+
// If our inputSubstType is also async and we are converting from a
5771+
// nonisolated caller to a global actor from a global actor, attach the
5772+
// global actor for thunk so we mangle appropriately.
5773+
auto outputIsolation = outputSubstType->getIsolation();
5774+
if (outputIsolation.getKind() ==
5775+
FunctionTypeIsolation::Kind::GlobalActor &&
5776+
inputSubstType->getIsolation().getKind() ==
5777+
FunctionTypeIsolation::Kind::NonIsolatedCaller)
5778+
globalActorForThunk =
5779+
outputIsolation.getGlobalActorType()->getCanonicalType();
5780+
}
56985781
}
56995782

57005783
auto thunk = SGF.SGM.getOrCreateReabstractionThunk(

lib/Sema/TypeCheckConcurrency.cpp

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2728,9 +2728,16 @@ namespace {
27282728
diagnoseNonSendableParametersAndResult(toFnType);
27292729
break;
27302730

2731-
case FunctionTypeIsolation::Kind::NonIsolatedCaller:
27322731
case FunctionTypeIsolation::Kind::Parameter:
27332732
llvm_unreachable("invalid conversion");
2733+
2734+
case FunctionTypeIsolation::Kind::NonIsolatedCaller:
2735+
// Non isolated caller is always async. This can only occur if we
2736+
// are converting from an `@Sendable` representation to something
2737+
// else. So we need to just check that we diagnose non sendable
2738+
// parameters and results.
2739+
diagnoseNonSendableParametersAndResult(toFnType);
2740+
break;
27342741
}
27352742
break;
27362743
}

0 commit comments

Comments
 (0)