Merge pull request #41567 from rjmccall/remove-async-context-flags

rjmccall · web-flow · commit a005f4549e97 · 2022-02-25T23:37:23.000-05:00
Remove the Flags field from AsyncContext
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
