Skip to content

[Runtime] Remove FIXMEs and hacks for tail calls. #37461

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 8 additions & 4 deletions include/swift/ABI/Task.h
Original file line number Diff line number Diff line change
Expand Up @@ -113,13 +113,15 @@ class alignas(2 * alignof(void*)) Job :
/// Given that we've fully established the job context in the current
/// thread, actually start running this job. To establish the context
/// correctly, call swift_job_run or runJobInExecutorContext.
SWIFT_CC(swiftasync)
void runInFullyEstablishedContext();

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

Expand Down Expand Up @@ -260,8 +262,9 @@ class AsyncTask : public Job {
/// in the current thread, start running this task. To establish
/// the job context correctly, call swift_job_run or
/// runInExecutorContext.
SWIFT_CC(swiftasync)
void runInFullyEstablishedContext() {
ResumeTask(ResumeContext);
return ResumeTask(ResumeContext); // 'return' forces tail call
}

/// Check whether this task has been cancelled.
Expand Down Expand Up @@ -534,11 +537,12 @@ static_assert(alignof(AsyncTask) == 2 * alignof(void*),
static_assert(offsetof(AsyncTask, Id) == 4 * sizeof(void *) + 4,
"AsyncTask::Id offset is wrong");

SWIFT_CC(swiftasync)
inline void Job::runInFullyEstablishedContext() {
if (auto task = dyn_cast<AsyncTask>(this))
task->runInFullyEstablishedContext();
return task->runInFullyEstablishedContext(); // 'return' forces tail call
else
runSimpleInFullyEstablishedContext();
return runSimpleInFullyEstablishedContext(); // 'return' forces tail call
}

/// An asynchronous context within a task. Generally contexts are
Expand Down
30 changes: 10 additions & 20 deletions stdlib/public/Concurrency/Actor.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1483,6 +1483,7 @@ static void swift_job_runImpl(Job *job, ExecutorRef executor) {
/// the actor lives for the duration of job execution.
/// Note that this may conflict with the retain/release
/// design in the DefaultActorImpl, but it does fix bugs!
SWIFT_CC(swiftasync)
static void processDefaultActor(DefaultActorImpl *currentActor,
RunningJobInfo runner) {
#if SWIFT_TASK_PRINTF_DEBUG
Expand Down Expand Up @@ -1557,6 +1558,7 @@ static void processDefaultActor(DefaultActorImpl *currentActor,
swift_release(actor);
}

SWIFT_CC(swiftasync)
void ProcessInlineJob::process(Job *job) {
DefaultActorImpl *actor = DefaultActorImpl::fromInlineJob(job);

Expand All @@ -1565,11 +1567,11 @@ void ProcessInlineJob::process(Job *job) {
auto targetPriority = job->getPriority();
auto runner = RunningJobInfo::forInline(targetPriority);

// FIXME: force tail call
swift_retain(actor);
return processDefaultActor(actor, runner);
return processDefaultActor(actor, runner); // 'return' forces tail call
}

SWIFT_CC(swiftasync)
void ProcessOutOfLineJob::process(Job *job) {
auto self = cast<ProcessOutOfLineJob>(job);
DefaultActorImpl *actor = self->Actor;
Expand All @@ -1581,21 +1583,20 @@ void ProcessOutOfLineJob::process(Job *job) {

delete self;

// FIXME: force tail call
swift_retain(actor);
return processDefaultActor(actor, runner);
return processDefaultActor(actor, runner); // 'return' forces tail call
}

SWIFT_CC(swiftasync)
void ProcessOverrideJob::process(Job *job) {
auto self = cast<ProcessOverrideJob>(job);

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

// FIXME: force tail call
swift_retain(actor);
return processDefaultActor(actor, runner);
return processDefaultActor(actor, runner); // 'return' forces tail call
}

void DefaultActorImpl::enqueue(Job *job) {
Expand Down Expand Up @@ -1800,13 +1801,6 @@ static bool tryAssumeThreadForSwitch(ExecutorRef newExecutor,
return false;
}

__attribute__((noinline))
SWIFT_CC(swiftasync)
static void force_tail_call_hack(AsyncTask *task) {
// This *should* be executed as a tail call.
return task->runInFullyEstablishedContext();
}

/// Given that we've assumed control of an executor on this thread,
/// continue to run the given task on it.
SWIFT_CC(swiftasync)
Expand All @@ -1819,10 +1813,7 @@ static void runOnAssumedThread(AsyncTask *task, ExecutorRef executor,
if (oldTracking) {
oldTracking->setActiveExecutor(executor);

// FIXME: force tail call
// return task->runInFullyEstablishedContext();
// This hack "ensures" that this call gets executed as a tail call.
return force_tail_call_hack(task);
return task->runInFullyEstablishedContext(); // 'return' forces tail call
}

// Otherwise, set up tracking info.
Expand Down Expand Up @@ -1865,8 +1856,7 @@ static void swift_task_switchImpl(SWIFT_ASYNC_CONTEXT AsyncContext *resumeContex
// we can just immediately continue running with the resume function
// we were passed in.
if (!currentExecutor.mustSwitchToRun(newExecutor)) {
// FIXME: force tail call
return resumeFunction(resumeContext);
return resumeFunction(resumeContext); // 'return' forces tail call
}

auto task = swift_task_getCurrent();
Expand All @@ -1890,7 +1880,7 @@ static void swift_task_switchImpl(SWIFT_ASYNC_CONTEXT AsyncContext *resumeContex
fprintf(stderr, "[%p] switch succeeded, task %p assumed thread for executor %p\n", pthread_self(), task, newExecutor.getIdentity());
#endif
giveUpThreadForSwitch(currentExecutor, runner);
// FIXME: force tail call
// 'return' forces tail call
return runOnAssumedThread(task, newExecutor, trackingInfo, runner);
}

Expand Down