Remove the Flags field from AsyncContext.

rjmccall · rjmccall · commit aca744b21165 · 2022-02-25T16:57:49.000-05:00
Generated code has never actually initialized this field, so we
might as well remove it.  Doing so mostly doesn't impact the ABI
since we don't store anything for arguments or results in the
context as part of the normal call sequence.  We do need to adjust
some of the hard-coded contexts, however, such as continuation
contexts and the statically-sized context for special runtime
async functions.
diff --git a/include/swift/ABI/MetadataValues.h b/include/swift/ABI/MetadataValues.h
@@ -2201,57 +2201,6 @@ class TaskOptionRecordFlags : public FlagSet<size_t> {
                                  getKind, setKind)
 };
 
-/// Kinds of async context.
-enum class AsyncContextKind {
-  /// An ordinary asynchronous function.
-  Ordinary         = 0,
-
-  /// A context which can yield to its caller.
-  Yielding         = 1,
-
-  /// A continuation context.
-  Continuation     = 2,
-
-  // Other kinds are reserved for interesting special
-  // intermediate contexts.
-
-  // Kinds >= 192 are private to the implementation.
-  First_Reserved = 192
-};
-
-/// Flags for async contexts.
-class AsyncContextFlags : public FlagSet<uint32_t> {
-public:
-  enum {
-    Kind                = 0,
-    Kind_width          = 8,
-
-    CanThrow            = 8,
-
-    // Kind-specific flags should grow down from 31.
-
-    Continuation_IsExecutorSwitchForced = 31,
-  };
-
-  explicit AsyncContextFlags(uint32_t bits) : FlagSet(bits) {}
-  constexpr AsyncContextFlags() {}
-  AsyncContextFlags(AsyncContextKind kind) {
-    setKind(kind);
-  }
-
-  /// The kind of context this represents.
-  FLAGSET_DEFINE_FIELD_ACCESSORS(Kind, Kind_width, AsyncContextKind,
-                                 getKind, setKind)
-
-  /// Whether this context is permitted to throw.
-  FLAGSET_DEFINE_FLAG_ACCESSORS(CanThrow, canThrow, setCanThrow)
-
-  /// See AsyncContinuationFlags::isExecutorSwitchForced.
-  FLAGSET_DEFINE_FLAG_ACCESSORS(Continuation_IsExecutorSwitchForced,
-                                continuation_isExecutorSwitchForced,
-                                continuation_setIsExecutorSwitchForced)
-};
-
 /// Flags passed to swift_continuation_init.
 class AsyncContinuationFlags : public FlagSet<size_t> {
 public:
diff --git a/include/swift/ABI/Task.h b/include/swift/ABI/Task.h
@@ -631,19 +631,9 @@ class alignas(MaximumAlignment) AsyncContext {
   TaskContinuationFunction * __ptrauth_swift_async_context_resume
     ResumeParent;
 
-  /// Flags describing this context.
-  ///
-  /// Note that this field is only 32 bits; any alignment padding
-  /// following this on 64-bit platforms can be freely used by the
-  /// function.  If the function is a yielding function, that padding
-  /// is of course interrupted by the YieldToParent field.
-  AsyncContextFlags Flags;
-
-  AsyncContext(AsyncContextFlags flags,
-               TaskContinuationFunction *resumeParent,
+  AsyncContext(TaskContinuationFunction *resumeParent,
                AsyncContext *parent)
-    : Parent(parent), ResumeParent(resumeParent),
-      Flags(flags) {}
+    : Parent(parent), ResumeParent(resumeParent) {}
 
   AsyncContext(const AsyncContext &) = delete;
   AsyncContext &operator=(const AsyncContext &) = delete;
@@ -667,48 +657,66 @@ class YieldingAsyncContext : public AsyncContext {
   TaskContinuationFunction * __ptrauth_swift_async_context_yield
     YieldToParent;
 
-  YieldingAsyncContext(AsyncContextFlags flags,
-                       TaskContinuationFunction *resumeParent,
+  YieldingAsyncContext(TaskContinuationFunction *resumeParent,
                        TaskContinuationFunction *yieldToParent,
                        AsyncContext *parent)
-    : AsyncContext(flags, resumeParent, parent),
+    : AsyncContext(resumeParent, parent),
       YieldToParent(yieldToParent) {}
-
-  static bool classof(const AsyncContext *context) {
-    return context->Flags.getKind() == AsyncContextKind::Yielding;
-  }
 };
 
 /// An async context that can be resumed as a continuation.
 class ContinuationAsyncContext : public AsyncContext {
 public:
+  class FlagsType : public FlagSet<size_t> {
+  public:
+    enum {
+      CanThrow = 0,
+      IsExecutorSwitchForced = 1,
+    };
+
+    explicit FlagsType(size_t bits) : FlagSet(bits) {}
+    constexpr FlagsType() {}
+
+    /// Whether this is a throwing continuation.
+    FLAGSET_DEFINE_FLAG_ACCESSORS(CanThrow,
+                                  canThrow,
+                                  setCanThrow)
+
+    /// See AsyncContinuationFlags::isExecutorSwitchForced().
+    FLAGSET_DEFINE_FLAG_ACCESSORS(IsExecutorSwitchForced,
+                                  isExecutorSwitchForced,
+                                  setIsExecutorSwitchForced)
+  };
+
+  /// Flags for the continuation.  Not public ABI.
+  FlagsType Flags;
+
   /// An atomic object used to ensure that a continuation is not
   /// scheduled immediately during a resume if it hasn't yet been
-  /// awaited by the function which set it up.
+  /// awaited by the function which set it up.  Not public ABI.
   std::atomic<ContinuationStatus> AwaitSynchronization;
 
   /// The error result value of the continuation.
   /// This should be null-initialized when setting up the continuation.
   /// Throwing resumers must overwrite this with a non-null value.
+  /// Public ABI.
   SwiftError *ErrorResult;
 
   /// A pointer to the normal result value of the continuation.
   /// Normal resumers must initialize this before resuming.
+  /// Public ABI.
   OpaqueValue *NormalResult;
 
   /// The executor that should be resumed to.
+  /// Public ABI.
   ExecutorRef ResumeToExecutor;
 
   void setErrorResult(SwiftError *error) {
     ErrorResult = error;
   }
 
   bool isExecutorSwitchForced() const {
-    return Flags.continuation_isExecutorSwitchForced();
-  }
-
-  static bool classof(const AsyncContext *context) {
-    return context->Flags.getKind() == AsyncContextKind::Continuation;
+    return Flags.isExecutorSwitchForced();
   }
 };
 
diff --git a/include/swift/Reflection/RuntimeInternals.h b/include/swift/Reflection/RuntimeInternals.h
@@ -131,7 +131,6 @@ template <typename Runtime>
 struct AsyncContext {
   typename Runtime::StoredSignedPointer Parent;
   typename Runtime::StoredSignedPointer ResumeParent;
-  uint32_t Flags;
 };
 
 template <typename Runtime>
diff --git a/lib/IRGen/GenCall.cpp b/lib/IRGen/GenCall.cpp
@@ -93,7 +93,7 @@ AsyncContextLayout irgen::getAsyncContextLayout(IRGenModule &IGM,
 }
 
 static Size getAsyncContextHeaderSize(IRGenModule &IGM) {
-  return 2 * IGM.getPointerSize() + Size(4);
+  return 2 * IGM.getPointerSize();
 }
 
 AsyncContextLayout irgen::getAsyncContextLayout(
@@ -123,19 +123,6 @@ AsyncContextLayout irgen::getAsyncContextLayout(
     typeInfos.push_back(&ti);
   }
 
-  // AsyncContextFlags Flags;
-  // FIXME: this appears to be dead; we should adjust the layout of
-  // the special async contexts that assume its existence and then
-  // remove it.
-  {
-    auto ty = SILType::getPrimitiveObjectType(
-        BuiltinIntegerType::get(32, IGM.IRGen.SIL.getASTContext())
-            ->getCanonicalType());
-    const auto &ti = IGM.getTypeInfo(ty);
-    valTypes.push_back(ty);
-    typeInfos.push_back(&ti);
-  }
-
   return AsyncContextLayout(IGM, LayoutStrategy::Optimal, valTypes, typeInfos,
                             originalType, substitutedType, substitutionMap);
 }
@@ -181,11 +168,7 @@ FunctionPointerKind::getStaticAsyncContextSize(IRGenModule &IGM) const {
     // If you add a new special runtime function, it is highly recommended
     // that you make calls to it allocate a little more memory than this!
     // These frames being this small is very arguably a mistake.
-    //
-    // FIXME: if we remove Flags in AsyncContextLayout (and thus from
-    // headerSize), account for it here so that we continue to pass the
-    // right amount of memory.
-    return headerSize + 2 * IGM.getPointerSize();
+    return headerSize + 3 * IGM.getPointerSize();
   }
   llvm_unreachable("covered switch");
 }
diff --git a/lib/IRGen/IRGenFunction.cpp b/lib/IRGen/IRGenFunction.cpp
@@ -575,6 +575,13 @@ static Address emitAddrOfContinuationNormalResultPointer(IRGenFunction &IGF,
                                                          Address context) {
   assert(context.getType() == IGF.IGM.ContinuationAsyncContextPtrTy);
   auto offset = 5 * IGF.IGM.getPointerSize();
+  return IGF.Builder.CreateStructGEP(context, 4, offset);
+}
+
+static Address emitAddrOfContinuationErrorResultPointer(IRGenFunction &IGF,
+                                                        Address context) {
+  assert(context.getType() == IGF.IGM.ContinuationAsyncContextPtrTy);
+  auto offset = 4 * IGF.IGM.getPointerSize();
   return IGF.Builder.CreateStructGEP(context, 3, offset);
 }
 
@@ -693,7 +700,8 @@ void IRGenFunction::emitAwaitAsyncContinuation(
     Explosion &outDirectResult, llvm::BasicBlock *&normalBB,
     llvm::PHINode *&optionalErrorResult, llvm::BasicBlock *&optionalErrorBB) {
   assert(AsyncCoroutineCurrentContinuationContext && "no active continuation");
-  auto pointerAlignment = IGM.getPointerAlignment();
+  Address continuationContext(AsyncCoroutineCurrentContinuationContext,
+                              IGM.getAsyncContextAlignment());
 
   // Call swift_continuation_await to check whether the continuation
   // has already been resumed.
@@ -703,11 +711,9 @@ void IRGenFunction::emitAwaitAsyncContinuation(
   // swift_continuation_await, emit the old inline sequence.  This can
   // be removed as soon as we're sure that such SDKs don't exist.
   if (!useContinuationAwait) {
-    auto contAwaitSyncAddr = Builder.CreateStructGEP(
-        AsyncCoroutineCurrentContinuationContext->getType()
-            ->getScalarType()
-            ->getPointerElementType(),
-        AsyncCoroutineCurrentContinuationContext, 1);
+    auto contAwaitSyncAddr =
+      Builder.CreateStructGEP(continuationContext, 2,
+                              3 * IGM.getPointerSize()).getAddress();
 
     auto pendingV = llvm::ConstantInt::get(
         contAwaitSyncAddr->getType()->getPointerElementType(),
@@ -759,11 +765,11 @@ void IRGenFunction::emitAwaitAsyncContinuation(
         Builder.CreateBitOrPointerCast(awaitFnPtr, IGM.Int8PtrTy));
 
     if (useContinuationAwait) {
-      arguments.push_back(AsyncCoroutineCurrentContinuationContext);
+      arguments.push_back(continuationContext.getAddress());
     } else {
       arguments.push_back(AsyncCoroutineCurrentResume);
       arguments.push_back(Builder.CreateBitOrPointerCast(
-        AsyncCoroutineCurrentContinuationContext, IGM.Int8PtrTy));
+        continuationContext.getAddress(), IGM.Int8PtrTy));
     }
 
     auto resultTy =
@@ -777,12 +783,7 @@ void IRGenFunction::emitAwaitAsyncContinuation(
   if (optionalErrorBB) {
     auto normalContBB = createBasicBlock("await.async.normal");
     auto contErrResultAddr =
-        Address(Builder.CreateStructGEP(
-                    AsyncCoroutineCurrentContinuationContext->getType()
-                        ->getScalarType()
-                        ->getPointerElementType(),
-                    AsyncCoroutineCurrentContinuationContext, 2),
-                pointerAlignment);
+        emitAddrOfContinuationErrorResultPointer(*this, continuationContext);
     auto errorRes = Builder.CreateLoad(contErrResultAddr);
     auto nullError = llvm::Constant::getNullValue(errorRes->getType());
     auto hasError = Builder.CreateICmpNE(errorRes, nullError);
@@ -795,13 +796,10 @@ void IRGenFunction::emitAwaitAsyncContinuation(
   // result slot, load from the temporary we created during
   // get_async_continuation.
   if (!isIndirectResult) {
-    auto contResultAddrAddr = Builder.CreateStructGEP(
-        AsyncCoroutineCurrentContinuationContext->getType()
-            ->getScalarType()
-            ->getPointerElementType(),
-        AsyncCoroutineCurrentContinuationContext, 3);
+    auto contResultAddrAddr =
+        emitAddrOfContinuationNormalResultPointer(*this, continuationContext);
     auto resultAddrVal =
-        Builder.CreateLoad(Address(contResultAddrAddr, pointerAlignment));
+        Builder.CreateLoad(contResultAddrAddr);
     // Take the result.
     auto &resumeTI = cast<LoadableTypeInfo>(getTypeInfo(resumeTy));
     auto resultStorageTy = resumeTI.getStorageType();
diff --git a/lib/IRGen/IRGenModule.cpp b/lib/IRGen/IRGenModule.cpp
@@ -662,9 +662,8 @@ IRGenModule::IRGenModule(IRGenerator &irgen,
   TaskContinuationFunctionPtrTy = TaskContinuationFunctionTy->getPointerTo();
 
   SwiftContextTy->setBody({
-    SwiftContextPtrTy,    // Parent
-    TaskContinuationFunctionPtrTy, // ResumeParent,
-    SizeTy,               // Flags
+    SwiftContextPtrTy,             // Parent
+    TaskContinuationFunctionPtrTy, // ResumeParent
   });
 
   AsyncTaskAndContextTy = createStructType(
@@ -674,6 +673,7 @@ IRGenModule::IRGenModule(IRGenerator &irgen,
   ContinuationAsyncContextTy = createStructType(
       *this, "swift.continuation_context",
       {SwiftContextTy,       // AsyncContext header
+       SizeTy,               // flags
        SizeTy,               // await synchronization
        ErrorPtrTy,           // error result pointer
        OpaquePtrTy,          // normal result address
diff --git a/stdlib/public/Concurrency/Task.cpp b/stdlib/public/Concurrency/Task.cpp
@@ -165,7 +165,8 @@ void NullaryContinuationJob::process(Job *_job) {
 
   _swift_task_dealloc_specific(task, job);
 
-  auto *context = cast<ContinuationAsyncContext>(continuation->ResumeContext);
+  auto *context =
+    static_cast<ContinuationAsyncContext*>(continuation->ResumeContext);
 
   context->setErrorResult(nullptr);
   swift_continuation_resume(continuation);
@@ -825,7 +826,6 @@ static AsyncTaskAndContext swift_task_create_commonImpl(
   // as if they might be null, even though the only time they ever might
   // be is the final hop.  Store a signed null instead.
   initialContext->Parent = nullptr;
-  initialContext->Flags = AsyncContextKind::Ordinary;
 
   concurrency::trace::task_create(task, parent, group, asyncLet);
 
@@ -1026,13 +1026,13 @@ swift_task_enqueueTaskOnExecutorImpl(AsyncTask *task, ExecutorRef executor)
 SWIFT_CC(swift)
 static AsyncTask *swift_continuation_initImpl(ContinuationAsyncContext *context,
                                               AsyncContinuationFlags flags) {
-  context->Flags = AsyncContextKind::Continuation;
+  context->Flags = ContinuationAsyncContext::FlagsType();
   if (flags.canThrow()) context->Flags.setCanThrow(true);
   if (flags.isExecutorSwitchForced())
-    context->Flags.continuation_setIsExecutorSwitchForced(true);
+    context->Flags.setIsExecutorSwitchForced(true);
   context->ErrorResult = nullptr;
 
-  // Set the current executor as the target executor unless there's
+  // Set the generic executor as the target executor unless there's
   // an executor override.
   if (!flags.hasExecutorOverride())
     context->ResumeToExecutor = ExecutorRef::generic();
@@ -1156,21 +1156,21 @@ static void resumeTaskAfterContinuation(AsyncTask *task,
 
 SWIFT_CC(swift)
 static void swift_continuation_resumeImpl(AsyncTask *task) {
-  auto context = cast<ContinuationAsyncContext>(task->ResumeContext);
+  auto context = static_cast<ContinuationAsyncContext*>(task->ResumeContext);
   resumeTaskAfterContinuation(task, context);
 }
 
 SWIFT_CC(swift)
 static void swift_continuation_throwingResumeImpl(AsyncTask *task) {
-  auto context = cast<ContinuationAsyncContext>(task->ResumeContext);
+  auto context = static_cast<ContinuationAsyncContext*>(task->ResumeContext);
   resumeTaskAfterContinuation(task, context);
 }
 
 
 SWIFT_CC(swift)
 static void swift_continuation_throwingResumeWithErrorImpl(AsyncTask *task,
                                                 /* +1 */ SwiftError *error) {
-  auto context = cast<ContinuationAsyncContext>(task->ResumeContext);
+  auto context = static_cast<ContinuationAsyncContext*>(task->ResumeContext);
   context->ErrorResult = error;
   resumeTaskAfterContinuation(task, context);
 }
diff --git a/stdlib/public/Concurrency/TaskPrivate.h b/stdlib/public/Concurrency/TaskPrivate.h
@@ -145,9 +145,16 @@ namespace {
 ///
 class TaskFutureWaitAsyncContext : public AsyncContext {
 public:
+  // The ABI reserves three words of storage for these contexts, which
+  // we currently use as follows.  These fields are not accessed by
+  // generated code; they're purely internal to the runtime, and only
+  // when the calling task actually suspends.
+  //
+  // (If you think three words is an odd choice, one of them used to be
+  // the context flags.)
   SwiftError *errorResult;
-
   OpaqueValue *successResultPointer;
+  void *_reserved;
 
   void fillWithSuccess(AsyncTask::FutureFragment *future) {
     fillWithSuccess(future->getStoragePtr(), future->getResultType(),
diff --git a/test/Distributed/distributed_actor_accessor_thunks_32bit.swift b/test/Distributed/distributed_actor_accessor_thunks_32bit.swift
diff --git a/test/Distributed/distributed_actor_accessor_thunks_64bit.swift b/test/Distributed/distributed_actor_accessor_thunks_64bit.swift
diff --git a/test/IRGen/async/builtins.sil b/test/IRGen/async/builtins.sil
diff --git a/test/IRGen/async/get_async_continuation.sil b/test/IRGen/async/get_async_continuation.sil
