Skip to content

Commit 88b4523

Browse files
Merge pull request #37485 from aschwaighofer/tail_call_fix_2nd_swift_task_switch_impl_5.5-05142021
[5.5-05142021] Make sure we tail call optimize a call in concurrency runtime's switch_task_impl.
2 parents bd9d775 + 684dbe2 commit 88b4523

File tree

3 files changed

+78
-15
lines changed

3 files changed

+78
-15
lines changed

include/swift/ABI/Task.h

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -99,13 +99,15 @@ class alignas(2 * alignof(void*)) Job : public HeapObject {
9999
/// Given that we've fully established the job context in the current
100100
/// thread, actually start running this job. To establish the context
101101
/// correctly, call swift_job_run or runJobInExecutorContext.
102+
SWIFT_CC(swiftasync)
102103
void runInFullyEstablishedContext();
103104

104105
/// Given that we've fully established the job context in the
105106
/// current thread, and that the job is a simple (non-task) job,
106107
/// actually start running this job.
108+
SWIFT_CC(swiftasync)
107109
void runSimpleInFullyEstablishedContext() {
108-
RunJob(this);
110+
return RunJob(this); // 'return' forces tail call
109111
}
110112
};
111113

@@ -224,8 +226,9 @@ class AsyncTask : public Job {
224226
/// in the current thread, start running this task. To establish
225227
/// the job context correctly, call swift_job_run or
226228
/// runInExecutorContext.
229+
SWIFT_CC(swiftasync)
227230
void runInFullyEstablishedContext() {
228-
ResumeTask(ResumeContext);
231+
return ResumeTask(ResumeContext); // 'return' forces tail call
229232
}
230233

231234
/// Check whether this task has been cancelled.
@@ -486,11 +489,12 @@ static_assert(sizeof(AsyncTask) == 14 * sizeof(void*),
486489
static_assert(alignof(AsyncTask) == 2 * alignof(void*),
487490
"AsyncTask alignment is wrong");
488491

492+
SWIFT_CC(swiftasync)
489493
inline void Job::runInFullyEstablishedContext() {
490494
if (auto task = dyn_cast<AsyncTask>(this))
491-
task->runInFullyEstablishedContext();
495+
return task->runInFullyEstablishedContext(); // 'return' forces tail call
492496
else
493-
runSimpleInFullyEstablishedContext();
497+
return runSimpleInFullyEstablishedContext(); // 'return' forces tail call
494498
}
495499

496500
/// An asynchronous context within a task. Generally contexts are

stdlib/public/Concurrency/Actor.cpp

Lines changed: 10 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1473,6 +1473,7 @@ static void swift_job_runImpl(Job *job, ExecutorRef executor) {
14731473
/// the actor lives for the duration of job execution.
14741474
/// Note that this may conflict with the retain/release
14751475
/// design in the DefaultActorImpl, but it does fix bugs!
1476+
SWIFT_CC(swiftasync)
14761477
static void processDefaultActor(DefaultActorImpl *currentActor,
14771478
RunningJobInfo runner) {
14781479
#if SWIFT_TASK_PRINTF_DEBUG
@@ -1547,6 +1548,7 @@ static void processDefaultActor(DefaultActorImpl *currentActor,
15471548
swift_release(actor);
15481549
}
15491550

1551+
SWIFT_CC(swiftasync)
15501552
void ProcessInlineJob::process(Job *job) {
15511553
DefaultActorImpl *actor = DefaultActorImpl::fromInlineJob(job);
15521554

@@ -1555,11 +1557,11 @@ void ProcessInlineJob::process(Job *job) {
15551557
auto targetPriority = job->getPriority();
15561558
auto runner = RunningJobInfo::forInline(targetPriority);
15571559

1558-
// FIXME: force tail call
15591560
swift_retain(actor);
1560-
return processDefaultActor(actor, runner);
1561+
return processDefaultActor(actor, runner); // 'return' forces tail call
15611562
}
15621563

1564+
SWIFT_CC(swiftasync)
15631565
void ProcessOutOfLineJob::process(Job *job) {
15641566
auto self = cast<ProcessOutOfLineJob>(job);
15651567
DefaultActorImpl *actor = self->Actor;
@@ -1571,21 +1573,20 @@ void ProcessOutOfLineJob::process(Job *job) {
15711573

15721574
delete self;
15731575

1574-
// FIXME: force tail call
15751576
swift_retain(actor);
1576-
return processDefaultActor(actor, runner);
1577+
return processDefaultActor(actor, runner); // 'return' forces tail call
15771578
}
15781579

1580+
SWIFT_CC(swiftasync)
15791581
void ProcessOverrideJob::process(Job *job) {
15801582
auto self = cast<ProcessOverrideJob>(job);
15811583

15821584
// Pull the actor and priority out of the job.
15831585
auto actor = self->Actor;
15841586
auto runner = RunningJobInfo::forOverride(self);
15851587

1586-
// FIXME: force tail call
15871588
swift_retain(actor);
1588-
return processDefaultActor(actor, runner);
1589+
return processDefaultActor(actor, runner); // 'return' forces tail call
15891590
}
15901591

15911592
void DefaultActorImpl::enqueue(Job *job) {
@@ -1814,8 +1815,7 @@ static void runOnAssumedThread(AsyncTask *task, ExecutorRef executor,
18141815
if (oldTracking) {
18151816
oldTracking->setActiveExecutor(executor);
18161817

1817-
// FIXME: force tail call
1818-
return task->runInFullyEstablishedContext();
1818+
return task->runInFullyEstablishedContext(); // 'return' forces tail call
18191819
}
18201820

18211821
// Otherwise, set up tracking info.
@@ -1858,8 +1858,7 @@ static void swift_task_switchImpl(SWIFT_ASYNC_CONTEXT AsyncContext *resumeContex
18581858
// we can just immediately continue running with the resume function
18591859
// we were passed in.
18601860
if (!currentExecutor.mustSwitchToRun(newExecutor)) {
1861-
// FIXME: force tail call
1862-
return resumeFunction(resumeContext);
1861+
return resumeFunction(resumeContext); // 'return' forces tail call
18631862
}
18641863

18651864
auto task = swift_task_getCurrent();
@@ -1883,7 +1882,7 @@ static void swift_task_switchImpl(SWIFT_ASYNC_CONTEXT AsyncContext *resumeContex
18831882
fprintf(stderr, "[%p] switch succeeded, task %p assumed thread for executor %p\n", pthread_self(), task, newExecutor.getIdentity());
18841883
#endif
18851884
giveUpThreadForSwitch(currentExecutor, runner);
1886-
// FIXME: force tail call
1885+
// 'return' forces tail call
18871886
return runOnAssumedThread(task, newExecutor, trackingInfo, runner);
18881887
}
18891888

test/Interpreter/async_fib.swift

Lines changed: 60 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,60 @@
1+
// RUN: %empty-directory(%t)
2+
// RUN: %target-build-swift -Xfrontend -enable-experimental-concurrency %s -parse-as-library -module-name main -o %t/main
3+
// RUN: %target-codesign %t/main
4+
// RUN: %target-run %t/main | %FileCheck %s
5+
6+
// REQUIRES: concurrency
7+
// REQUIRES: executable_test
8+
// UNSUPPORTED: use_os_stdlib
9+
// UNSUPPORTED: back_deployment_runtime
10+
// UNSUPPORTED: OS=windows-msvc
11+
12+
var gg = 0
13+
14+
@inline(never)
15+
public func identity<T>(_ x: T) -> T {
16+
gg += 1
17+
return x
18+
}
19+
20+
actor Actor {
21+
var x: Int = 0
22+
init(x: Int) { self.x = x }
23+
24+
@inline(never)
25+
func get(_ i: Int ) async -> Int {
26+
return i + x
27+
}
28+
}
29+
30+
// Used to crash with an stack overflow with m >= 18
31+
let m = 22
32+
33+
@inline(never)
34+
func asyncFib(_ n: Int, _ a1: Actor, _ a2: Actor) async -> Int {
35+
if n == 0 {
36+
return await a1.get(n)
37+
}
38+
if n == 1 {
39+
return await a2.get(n)
40+
}
41+
42+
let first = await asyncFib(n-2, a1, a2)
43+
let second = await asyncFib(n-1, a1, a2)
44+
45+
let result = first + second
46+
47+
return result
48+
}
49+
50+
@main struct Main {
51+
static func main() async {
52+
let a1 = Actor(x: 0)
53+
let a2 = Actor(x: 0)
54+
_ = await asyncFib(identity(m), a1, a2)
55+
56+
// CHECK: result: 0
57+
await print("result: \(a1.x)");
58+
await print(a2.x)
59+
}
60+
}

0 commit comments

Comments
 (0)