Skip to content

Commit 48b729d

Browse files
slavapestovslavapestov
authored
Merge pull request swiftlang#36641 from slavapestov/more-reasync-fixes
More 'reasync' fixes
2 parents 8e401d2 + 3cc0d36 commit 48b729d

File tree

12 files changed

+163
-90
lines changed

12 files changed

+163
-90
lines changed

‎include/swift/SIL/ApplySite.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -452,7 +452,7 @@ class ApplySite {
452452
case ApplySiteKind::TryApplyInst:
453453
return cast<TryApplyInst>(Inst)->getApplyOptions();
454454
case ApplySiteKind::PartialApplyInst:
455-
llvm_unreachable("Unhandled case");
455+
return ApplyOptions();
456456
}
457457
}
458458
/// Return whether the given apply is of a formally-throwing function

‎include/swift/SIL/TypeSubstCloner.h

Lines changed: 15 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -54,6 +54,7 @@ class TypeSubstCloner : public SILClonerWithScopes<ImplClass> {
5454
SubstitutionMap Subs;
5555
SmallVector<SILValue, 8> Args;
5656
SubstitutionMap RecursiveSubs;
57+
ApplyOptions ApplyOpts;
5758

5859
public:
5960
ApplySiteCloningHelper(ApplySite AI, TypeSubstCloner &Cloner)
@@ -67,6 +68,14 @@ class TypeSubstCloner : public SILClonerWithScopes<ImplClass> {
6768
// Remap substitutions.
6869
Subs = Cloner.getOpSubstitutionMap(AI.getSubstitutionMap());
6970

71+
// If we're inlining a [noasync] function, make sure any calls inside it
72+
// are marked as [noasync] as appropriate.
73+
ApplyOpts = AI.getApplyOptions();
74+
if (!Builder.getFunction().isAsync() &&
75+
SubstCalleeSILType.castTo<SILFunctionType>()->isAsync()) {
76+
ApplyOpts |= ApplyFlags::DoesNotAwait;
77+
}
78+
7079
if (!Cloner.Inlining) {
7180
FunctionRefInst *FRI = dyn_cast<FunctionRefInst>(AI.getCallee());
7281
if (FRI && FRI->getReferencedFunction() == AI.getFunction() &&
@@ -123,6 +132,10 @@ class TypeSubstCloner : public SILClonerWithScopes<ImplClass> {
123132
SubstitutionMap getSubstitutions() const {
124133
return Subs;
125134
}
135+
136+
ApplyOptions getApplyOptions() const {
137+
return ApplyOpts;
138+
}
126139
};
127140

128141
public:
@@ -214,7 +227,7 @@ class TypeSubstCloner : public SILClonerWithScopes<ImplClass> {
214227
getBuilder().createApply(getOpLocation(Inst->getLoc()),
215228
Helper.getCallee(), Helper.getSubstitutions(),
216229
Helper.getArguments(),
217-
Inst->getApplyOptions(),
230+
Helper.getApplyOptions(),
218231
GenericSpecializationInformation::create(
219232
Inst, getBuilder()));
220233
// Specialization can return noreturn applies that were not identified as
@@ -234,7 +247,7 @@ class TypeSubstCloner : public SILClonerWithScopes<ImplClass> {
234247
Helper.getSubstitutions(), Helper.getArguments(),
235248
getOpBasicBlock(Inst->getNormalBB()),
236249
getOpBasicBlock(Inst->getErrorBB()),
237-
Inst->getApplyOptions(),
250+
Helper.getApplyOptions(),
238251
GenericSpecializationInformation::create(
239252
Inst, getBuilder()));
240253
recordClonedInstruction(Inst, N);

‎lib/IRGen/LoadableByAddress.cpp

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2540,7 +2540,8 @@ void LoadableByAddress::recreateSingleApply(
25402540
applyBuilder.createTryApply(
25412541
castedApply->getLoc(), callee,
25422542
applySite.getSubstitutionMap(), callArgs,
2543-
castedApply->getNormalBB(), castedApply->getErrorBB());
2543+
castedApply->getNormalBB(), castedApply->getErrorBB(),
2544+
castedApply->getApplyOptions());
25442545
break;
25452546
}
25462547
case SILInstructionKind::BeginApplyInst: {

‎lib/SILOptimizer/Mandatory/MandatoryInlining.cpp

Lines changed: 74 additions & 73 deletions
Original file line numberDiff line numberDiff line change
@@ -610,6 +610,78 @@ static SILValue getLoadedCalleeValue(LoadInst *li) {
610610
return si->getSrc();
611611
}
612612

613+
// PartialApply/ThinToThick -> ConvertFunction patterns are generated
614+
// by @noescape closures.
615+
//
616+
// FIXME: We don't currently handle mismatched return types, however, this
617+
// would be a good optimization to handle and would be as simple as inserting
618+
// a cast.
619+
static SILValue stripFunctionConversions(SILValue CalleeValue) {
620+
// Skip any copies that we see.
621+
CalleeValue = stripCopiesAndBorrows(CalleeValue);
622+
623+
// We can also allow a thin @escape to noescape conversion as such:
624+
// %1 = function_ref @thin_closure_impl : $@convention(thin) () -> ()
625+
// %2 = convert_function %1 :
626+
// $@convention(thin) () -> () to $@convention(thin) @noescape () -> ()
627+
// %3 = thin_to_thick_function %2 :
628+
// $@convention(thin) @noescape () -> () to
629+
// $@noescape @callee_guaranteed () -> ()
630+
// %4 = apply %3() : $@noescape @callee_guaranteed () -> ()
631+
if (auto *ConvertFn = dyn_cast<ConvertFunctionInst>(CalleeValue)) {
632+
// If the conversion only changes the substitution level of the function,
633+
// we can also look through it.
634+
if (ConvertFn->onlyConvertsSubstitutions())
635+
return stripFunctionConversions(ConvertFn->getOperand());
636+
637+
auto FromCalleeTy =
638+
ConvertFn->getOperand()->getType().castTo<SILFunctionType>();
639+
if (FromCalleeTy->getExtInfo().hasContext())
640+
return CalleeValue;
641+
642+
auto ToCalleeTy = ConvertFn->getType().castTo<SILFunctionType>();
643+
auto EscapingCalleeTy = ToCalleeTy->getWithExtInfo(
644+
ToCalleeTy->getExtInfo().withNoEscape(false));
645+
if (FromCalleeTy != EscapingCalleeTy)
646+
return CalleeValue;
647+
648+
return stripCopiesAndBorrows(ConvertFn->getOperand());
649+
}
650+
651+
// Ignore mark_dependence users. A partial_apply [stack] uses them to mark
652+
// the dependence of the trivial closure context value on the captured
653+
// arguments.
654+
if (auto *MD = dyn_cast<MarkDependenceInst>(CalleeValue)) {
655+
while (MD) {
656+
CalleeValue = MD->getValue();
657+
MD = dyn_cast<MarkDependenceInst>(CalleeValue);
658+
}
659+
return CalleeValue;
660+
}
661+
662+
auto *CFI = dyn_cast<ConvertEscapeToNoEscapeInst>(CalleeValue);
663+
if (!CFI)
664+
return stripCopiesAndBorrows(CalleeValue);
665+
666+
// TODO: Handle argument conversion. All the code in this file needs to be
667+
// cleaned up and generalized. The argument conversion handling in
668+
// optimizeApplyOfConvertFunctionInst should apply to any combine
669+
// involving an apply, not just a specific pattern.
670+
//
671+
// For now, just handle conversion that doesn't affect argument types,
672+
// return types, or throws. We could trivially handle any other
673+
// representation change, but the only one that doesn't affect the ABI and
674+
// matters here is @noescape, so just check for that.
675+
auto FromCalleeTy = CFI->getOperand()->getType().castTo<SILFunctionType>();
676+
auto ToCalleeTy = CFI->getType().castTo<SILFunctionType>();
677+
auto EscapingCalleeTy =
678+
ToCalleeTy->getWithExtInfo(ToCalleeTy->getExtInfo().withNoEscape(false));
679+
if (FromCalleeTy != EscapingCalleeTy)
680+
return stripCopiesAndBorrows(CalleeValue);
681+
682+
return stripCopiesAndBorrows(CFI->getOperand());
683+
}
684+
613685
/// Returns the callee SILFunction called at a call site, in the case
614686
/// that the call is transparent (as in, both that the call is marked
615687
/// with the transparent flag and that callee function is actually transparently
@@ -646,79 +718,8 @@ getCalleeFunction(SILFunction *F, FullApplySite AI, bool &IsThick,
646718
CalleeValue = stripCopiesAndBorrows(CalleeValue);
647719
}
648720

649-
// PartialApply/ThinToThick -> ConvertFunction patterns are generated
650-
// by @noescape closures.
651-
//
652-
// FIXME: We don't currently handle mismatched return types, however, this
653-
// would be a good optimization to handle and would be as simple as inserting
654-
// a cast.
655-
auto skipFuncConvert = [](SILValue CalleeValue) {
656-
// Skip any copies that we see.
657-
CalleeValue = stripCopiesAndBorrows(CalleeValue);
658-
659-
// We can also allow a thin @escape to noescape conversion as such:
660-
// %1 = function_ref @thin_closure_impl : $@convention(thin) () -> ()
661-
// %2 = convert_function %1 :
662-
// $@convention(thin) () -> () to $@convention(thin) @noescape () -> ()
663-
// %3 = thin_to_thick_function %2 :
664-
// $@convention(thin) @noescape () -> () to
665-
// $@noescape @callee_guaranteed () -> ()
666-
// %4 = apply %3() : $@noescape @callee_guaranteed () -> ()
667-
if (auto *ConvertFn = dyn_cast<ConvertFunctionInst>(CalleeValue)) {
668-
// If the conversion only changes the substitution level of the function,
669-
// we can also look through it.
670-
if (ConvertFn->onlyConvertsSubstitutions()) {
671-
return stripCopiesAndBorrows(ConvertFn->getOperand());
672-
}
673-
674-
auto FromCalleeTy =
675-
ConvertFn->getOperand()->getType().castTo<SILFunctionType>();
676-
if (FromCalleeTy->getExtInfo().hasContext())
677-
return CalleeValue;
678-
auto ToCalleeTy = ConvertFn->getType().castTo<SILFunctionType>();
679-
auto EscapingCalleeTy = ToCalleeTy->getWithExtInfo(
680-
ToCalleeTy->getExtInfo().withNoEscape(false));
681-
if (FromCalleeTy != EscapingCalleeTy)
682-
return CalleeValue;
683-
return stripCopiesAndBorrows(ConvertFn->getOperand());
684-
}
685-
686-
// Ignore mark_dependence users. A partial_apply [stack] uses them to mark
687-
// the dependence of the trivial closure context value on the captured
688-
// arguments.
689-
if (auto *MD = dyn_cast<MarkDependenceInst>(CalleeValue)) {
690-
while (MD) {
691-
CalleeValue = MD->getValue();
692-
MD = dyn_cast<MarkDependenceInst>(CalleeValue);
693-
}
694-
return CalleeValue;
695-
}
696-
697-
auto *CFI = dyn_cast<ConvertEscapeToNoEscapeInst>(CalleeValue);
698-
if (!CFI)
699-
return stripCopiesAndBorrows(CalleeValue);
700-
701-
// TODO: Handle argument conversion. All the code in this file needs to be
702-
// cleaned up and generalized. The argument conversion handling in
703-
// optimizeApplyOfConvertFunctionInst should apply to any combine
704-
// involving an apply, not just a specific pattern.
705-
//
706-
// For now, just handle conversion that doesn't affect argument types,
707-
// return types, or throws. We could trivially handle any other
708-
// representation change, but the only one that doesn't affect the ABI and
709-
// matters here is @noescape, so just check for that.
710-
auto FromCalleeTy = CFI->getOperand()->getType().castTo<SILFunctionType>();
711-
auto ToCalleeTy = CFI->getType().castTo<SILFunctionType>();
712-
auto EscapingCalleeTy =
713-
ToCalleeTy->getWithExtInfo(ToCalleeTy->getExtInfo().withNoEscape(false));
714-
if (FromCalleeTy != EscapingCalleeTy)
715-
return stripCopiesAndBorrows(CalleeValue);
716-
717-
return stripCopiesAndBorrows(CFI->getOperand());
718-
};
719-
720721
// Look through a escape to @noescape conversion.
721-
CalleeValue = skipFuncConvert(CalleeValue);
722+
CalleeValue = stripFunctionConversions(CalleeValue);
722723

723724
// We are allowed to see through exactly one "partial apply" instruction or
724725
// one "thin to thick function" instructions, since those are the patterns
@@ -735,7 +736,7 @@ getCalleeFunction(SILFunction *F, FullApplySite AI, bool &IsThick,
735736
IsThick = true;
736737
}
737738

738-
CalleeValue = skipFuncConvert(CalleeValue);
739+
CalleeValue = stripFunctionConversions(CalleeValue);
739740

740741
auto *FRI = dyn_cast<FunctionRefInst>(CalleeValue);
741742
if (!FRI)

‎lib/Sema/ConstraintSystem.cpp

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2511,10 +2511,14 @@ FunctionType::ExtInfo ConstraintSystem::closureEffects(ClosureExpr *expr) {
25112511

25122512
bool walkToDeclPre(Decl *decl) override {
25132513
// Do not walk into function or type declarations.
2514-
if (!isa<PatternBindingDecl>(decl))
2515-
return false;
2514+
if (auto *patternBinding = dyn_cast<PatternBindingDecl>(decl)) {
2515+
if (patternBinding->isAsyncLet())
2516+
FoundAsync = true;
25162517

2517-
return true;
2518+
return true;
2519+
}
2520+
2521+
return false;
25182522
}
25192523

25202524
std::pair<bool, Stmt *> walkToStmtPre(Stmt *stmt) override {

‎lib/Sema/TypeCheckEffects.cpp

Lines changed: 3 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1153,17 +1153,14 @@ class ApplyClassifier {
11531153
return ShouldNotRecurse;
11541154
}
11551155
ShouldRecurse_t checkAsyncLet(PatternBindingDecl *patternBinding) {
1156-
// FIXME
1157-
llvm_unreachable("Test me");
1158-
// return ShouldRecurse;
1156+
AsyncKind = ConditionalEffectKind::Always;
1157+
return ShouldRecurse;
11591158
}
11601159
ShouldRecurse_t checkThrow(ThrowStmt *E) {
11611160
return ShouldRecurse;
11621161
}
11631162
ShouldRecurse_t checkInterpolatedStringLiteral(InterpolatedStringLiteralExpr *E) {
1164-
// FIXME
1165-
llvm_unreachable("Test me");
1166-
//return ShouldRecurse;
1163+
return ShouldRecurse;
11671164
}
11681165

11691166
ShouldRecurse_t checkIfConfig(IfConfigDecl *D) {

‎lib/Serialization/DeserializeSIL.cpp

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1551,7 +1551,8 @@ bool SILDeserializer::readSILInstruction(SILFunction *Fn,
15511551
SubstitutionMap Substitutions = MF->getSubstitutionMap(NumSubs);
15521552

15531553
ResultInst = Builder.createTryApply(Loc, getLocalValue(ValID, FnTy),
1554-
Substitutions, Args, normalBB, errorBB);
1554+
Substitutions, Args, normalBB, errorBB,
1555+
ApplyOpts);
15551556
break;
15561557
}
15571558
case SILInstructionKind::PartialApplyInst: {

‎test/Concurrency/Runtime/reasync.swift

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
// RUN: %empty-directory(%t)
2-
// RUN: %target-swift-frontend -emit-module-path %t/reasync.swiftmodule %S/Inputs/reasync.swift -enable-experimental-concurrency
2+
// RUN: %target-swift-frontend -emit-module-path %t/reasync.swiftmodule %S/Inputs/reasync.swift -enable-experimental-concurrency -verify-syntax-tree
33
// RUN: %target-build-swift %s -I %t -o %t/main -module-name main
44
// RUN: %target-run %t/main
55

‎test/Concurrency/reasync.swift

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -128,3 +128,22 @@ func invalidReasyncBody(_: () async -> ()) reasync {
128128
func validReasyncBody(_ fn: () async -> ()) reasync {
129129
await fn()
130130
}
131+
132+
//// String interpolation
133+
134+
func reasyncWithAutoclosure2(_: @autoclosure () async -> String) reasync {}
135+
136+
func callReasyncWithAutoclosure3() {
137+
let world = 123
138+
reasyncWithAutoclosure2("Hello \(world)")
139+
}
140+
141+
//// async let
142+
143+
func callReasyncWithAutoclosure4(_: () async -> ()) reasync {
144+
await reasyncFunction {
145+
async let x = 123
146+
147+
_ = await x
148+
}
149+
}

‎test/SILOptimizer/mandatory_inlining_generic_autoclosure.swift

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
// RUN: %target-swift-frontend -emit-sil %s | %FileCheck %s
2+
3+
@_transparent func takesGenericAutoclosure<T>(_ fn: @autoclosure () -> T) {
4+
_ = fn()
5+
}
6+
7+
// CHECK-LABEL: sil hidden @$s38mandatory_inlining_generic_autoclosure23callsGenericAutoclosureyyxlF : $@convention(thin) <T> (@in_guaranteed T) -> () {
8+
// CHECK-NOT: function_ref @$s38mandatory_inlining_generic_autoclosure23callsGenericAutoclosureyyxlFxyXEfu_
9+
10+
func callsGenericAutoclosure<T>(_ t: T) {
11+
takesGenericAutoclosure(t)
12+
}

0 commit comments

Comments
 (0)