[clang] [llvm] [Clang] Introduce [[clang::structured_concurrency]] (PR #94693)
via cfe-commits
cfe-commits at lists.llvm.org
Fri Jun 7 11:17:34 PDT 2024
llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT-->
@llvm/pr-subscribers-coroutines
Author: Yuxuan Chen (yuxuanchen1997)
<details>
<summary>Changes</summary>
---
Full diff: https://github.com/llvm/llvm-project/pull/94693.diff
10 Files Affected:
- (modified) clang/include/clang/Basic/Attr.td (+8)
- (modified) clang/include/clang/Basic/AttrDocs.td (+20)
- (modified) clang/lib/CodeGen/CGCoroutine.cpp (+71-5)
- (modified) clang/lib/CodeGen/CGExpr.cpp (+5-1)
- (modified) clang/lib/CodeGen/CodeGenFunction.h (+3)
- (modified) clang/test/Misc/pragma-attribute-supported-attributes-list.test (+1)
- (modified) llvm/include/llvm/IR/Intrinsics.td (+3)
- (modified) llvm/lib/Transforms/Coroutines/CoroCleanup.cpp (+8-3)
- (modified) llvm/lib/Transforms/Coroutines/CoroElide.cpp (+53-3)
- (modified) llvm/lib/Transforms/Coroutines/Coroutines.cpp (+1)
``````````diff
diff --git a/clang/include/clang/Basic/Attr.td b/clang/include/clang/Basic/Attr.td
index 17d9a710d948b..0fb623d6c0515 100644
--- a/clang/include/clang/Basic/Attr.td
+++ b/clang/include/clang/Basic/Attr.td
@@ -1212,6 +1212,14 @@ def CoroDisableLifetimeBound : InheritableAttr {
let SimpleHandler = 1;
}
+def CoroStructuredConcurrencyType : InheritableAttr {
+ let Spellings = [Clang<"coro_structured_concurrency">];
+ let Subjects = SubjectList<[CXXRecord]>;
+ let LangOpts = [CPlusPlus];
+ let Documentation = [CoroStructuredConcurrencyDoc];
+ let SimpleHandler = 1;
+}
+
// OSObject-based attributes.
def OSConsumed : InheritableParamAttr {
let Spellings = [Clang<"os_consumed">];
diff --git a/clang/include/clang/Basic/AttrDocs.td b/clang/include/clang/Basic/AttrDocs.td
index 70d5dfa8aaf86..50fc0fc2d16f6 100644
--- a/clang/include/clang/Basic/AttrDocs.td
+++ b/clang/include/clang/Basic/AttrDocs.td
@@ -8015,6 +8015,26 @@ but do not pass them to the underlying coroutine or pass them by value.
}];
}
+def CoroStructuredConcurrencyDoc : Documentation {
+ let Category = DocCatDecl;
+ let Content = [{
+The ``[[clang::coro_structured_concurrency]]`` is a class attribute which can be applied
+to a coroutine return type.
+
+When a coroutine function that returns such a type calls another coroutine function,
+the compiler performs heap allocation elision when the following conditions are all met:
+- callee coroutine function returns a type that is annotated with
+ ``[[clang::coro_structured_concurrency]]``.
+- The callee coroutine function is inlined.
+- In caller coroutine, the return value of the callee is a prvalue or an xvalue, and
+- The temporary expression containing the callee coroutine object is immediately co_awaited.
+
+The behavior is undefined if any of the following condition was met:
+- the caller coroutine is destroyed earlier than the callee coroutine.
+
+ }];
+}
+
def CountedByDocs : Documentation {
let Category = DocCatField;
let Content = [{
diff --git a/clang/lib/CodeGen/CGCoroutine.cpp b/clang/lib/CodeGen/CGCoroutine.cpp
index b4c724422c14a..78ae04982cba0 100644
--- a/clang/lib/CodeGen/CGCoroutine.cpp
+++ b/clang/lib/CodeGen/CGCoroutine.cpp
@@ -12,9 +12,12 @@
#include "CGCleanup.h"
#include "CodeGenFunction.h"
-#include "llvm/ADT/ScopeExit.h"
+#include "clang/AST/ExprCXX.h"
#include "clang/AST/StmtCXX.h"
#include "clang/AST/StmtVisitor.h"
+#include "llvm/ADT/ArrayRef.h"
+#include "llvm/ADT/ScopeExit.h"
+#include "llvm/IR/Intrinsics.h"
using namespace clang;
using namespace CodeGen;
@@ -219,17 +222,81 @@ namespace {
RValue RV;
};
}
+
+static MaterializeTemporaryExpr *
+getStructuredConcurrencyOperand(ASTContext &Ctx,
+ CoroutineSuspendExpr const &S) {
+ auto *E = S.getCommonExpr();
+ auto *Temporary = dyn_cast_or_null<MaterializeTemporaryExpr>(E);
+ if (!Temporary)
+ return nullptr;
+
+ auto *Operator =
+ dyn_cast_or_null<CXXOperatorCallExpr>(Temporary->getSubExpr());
+
+ if (!Operator ||
+ Operator->getOperator() != OverloadedOperatorKind::OO_Coawait ||
+ Operator->getNumArgs() != 1)
+ return nullptr;
+
+ Expr *Arg = Operator->getArg(0);
+ assert(Arg && "Arg to operator co_await should not be null");
+ auto *CalleeRetClass = Arg->getType()->getAsCXXRecordDecl();
+
+ if (!CalleeRetClass ||
+ !CalleeRetClass->hasAttr<CoroStructuredConcurrencyTypeAttr>())
+ return nullptr;
+
+ if (!Arg->isTemporaryObject(Ctx, CalleeRetClass)) {
+ return nullptr;
+ }
+
+ return dyn_cast<MaterializeTemporaryExpr>(Arg);
+}
+
static LValueOrRValue emitSuspendExpression(CodeGenFunction &CGF, CGCoroData &Coro,
CoroutineSuspendExpr const &S,
AwaitKind Kind, AggValueSlot aggSlot,
bool ignoreResult, bool forLValue) {
auto *E = S.getCommonExpr();
+ auto &Builder = CGF.Builder;
+ bool MarkOperandSafeIntrinsic = false;
+
+ auto *TemporaryOperand = [&]() -> MaterializeTemporaryExpr * {
+ bool CurFnRetTyHasAttr = false;
+ if (auto *RetTyPtr = CGF.FnRetTy.getTypePtrOrNull()) {
+ if (auto *CxxRecord = RetTyPtr->getAsCXXRecordDecl()) {
+ CurFnRetTyHasAttr =
+ CxxRecord->hasAttr<CoroStructuredConcurrencyTypeAttr>();
+ }
+ }
+
+ if (CurFnRetTyHasAttr) {
+ return getStructuredConcurrencyOperand(CGF.getContext(), S);
+ }
+ return nullptr;
+ }();
+
+ if (TemporaryOperand) {
+ CGF.TemporaryValues[TemporaryOperand] = nullptr;
+ MarkOperandSafeIntrinsic = true;
+ }
+
auto CommonBinder =
CodeGenFunction::OpaqueValueMappingData::bind(CGF, S.getOpaqueValue(), E);
auto UnbindCommonOnExit =
llvm::make_scope_exit([&] { CommonBinder.unbind(CGF); });
+ if (MarkOperandSafeIntrinsic) {
+ auto It = CGF.TemporaryValues.find(TemporaryOperand);
+ assert(It != CGF.TemporaryValues.end());
+ if (auto Value = It->second)
+ Builder.CreateCall(CGF.CGM.getIntrinsic(llvm::Intrinsic::coro_safe_elide),
+ {Value});
+ CGF.TemporaryValues.erase(TemporaryOperand);
+ }
+
auto Prefix = buildSuspendPrefixStr(Coro, Kind);
BasicBlock *ReadyBlock = CGF.createBasicBlock(Prefix + Twine(".ready"));
BasicBlock *SuspendBlock = CGF.createBasicBlock(Prefix + Twine(".suspend"));
@@ -241,7 +308,6 @@ static LValueOrRValue emitSuspendExpression(CodeGenFunction &CGF, CGCoroData &Co
// Otherwise, emit suspend logic.
CGF.EmitBlock(SuspendBlock);
- auto &Builder = CGF.Builder;
llvm::Function *CoroSave = CGF.CGM.getIntrinsic(llvm::Intrinsic::coro_save);
auto *NullPtr = llvm::ConstantPointerNull::get(CGF.CGM.Int8PtrTy);
auto *SaveCall = Builder.CreateCall(CoroSave, {NullPtr});
@@ -255,9 +321,9 @@ static LValueOrRValue emitSuspendExpression(CodeGenFunction &CGF, CGCoroData &Co
"expected to be called in coroutine context");
SmallVector<llvm::Value *, 3> SuspendIntrinsicCallArgs;
- SuspendIntrinsicCallArgs.push_back(
- CGF.getOrCreateOpaqueLValueMapping(S.getOpaqueValue()).getPointer(CGF));
-
+ auto *BoundAwaiterValue =
+ CGF.getOrCreateOpaqueLValueMapping(S.getOpaqueValue()).getPointer(CGF);
+ SuspendIntrinsicCallArgs.push_back(BoundAwaiterValue);
SuspendIntrinsicCallArgs.push_back(CGF.CurCoro.Data->CoroBegin);
SuspendIntrinsicCallArgs.push_back(SuspendWrapper);
diff --git a/clang/lib/CodeGen/CGExpr.cpp b/clang/lib/CodeGen/CGExpr.cpp
index d6478cc6835d8..139a602650b6d 100644
--- a/clang/lib/CodeGen/CGExpr.cpp
+++ b/clang/lib/CodeGen/CGExpr.cpp
@@ -618,7 +618,11 @@ EmitMaterializeTemporaryExpr(const MaterializeTemporaryExpr *M) {
}
}
- return MakeAddrLValue(Object, M->getType(), AlignmentSource::Decl);
+ auto Ret = MakeAddrLValue(Object, M->getType(), AlignmentSource::Decl);
+ if (TemporaryValues.contains(M)) {
+ TemporaryValues[M] = Ret.getPointer(*this);
+ }
+ return Ret;
}
RValue
diff --git a/clang/lib/CodeGen/CodeGenFunction.h b/clang/lib/CodeGen/CodeGenFunction.h
index 5739fbaaa9194..083cb8e2f376e 100644
--- a/clang/lib/CodeGen/CodeGenFunction.h
+++ b/clang/lib/CodeGen/CodeGenFunction.h
@@ -371,6 +371,9 @@ class CodeGenFunction : public CodeGenTypeCache {
};
CGCoroInfo CurCoro;
+ llvm::SmallDenseMap<const MaterializeTemporaryExpr *, llvm::Value *>
+ TemporaryValues;
+
bool isCoroutine() const {
return CurCoro.Data != nullptr;
}
diff --git a/clang/test/Misc/pragma-attribute-supported-attributes-list.test b/clang/test/Misc/pragma-attribute-supported-attributes-list.test
index 99732694f72a5..0369906d7c275 100644
--- a/clang/test/Misc/pragma-attribute-supported-attributes-list.test
+++ b/clang/test/Misc/pragma-attribute-supported-attributes-list.test
@@ -62,6 +62,7 @@
// CHECK-NEXT: CoroLifetimeBound (SubjectMatchRule_record)
// CHECK-NEXT: CoroOnlyDestroyWhenComplete (SubjectMatchRule_record)
// CHECK-NEXT: CoroReturnType (SubjectMatchRule_record)
+// CHECK-NEXT: CoroStructuredConcurrencyType (SubjectMatchRule_record)
// CHECK-NEXT: CoroWrapper (SubjectMatchRule_function)
// CHECK-NEXT: DLLExport (SubjectMatchRule_function, SubjectMatchRule_variable, SubjectMatchRule_record, SubjectMatchRule_objc_interface)
// CHECK-NEXT: DLLImport (SubjectMatchRule_function, SubjectMatchRule_variable, SubjectMatchRule_record, SubjectMatchRule_objc_interface)
diff --git a/llvm/include/llvm/IR/Intrinsics.td b/llvm/include/llvm/IR/Intrinsics.td
index 107442623ab7b..8e554d99ee3b6 100644
--- a/llvm/include/llvm/IR/Intrinsics.td
+++ b/llvm/include/llvm/IR/Intrinsics.td
@@ -1729,6 +1729,9 @@ def int_coro_subfn_addr : DefaultAttrsIntrinsic<
[IntrReadMem, IntrArgMemOnly, ReadOnly<ArgIndex<0>>,
NoCapture<ArgIndex<0>>]>;
+def int_coro_safe_elide : DefaultAttrsIntrinsic<
+ [], [llvm_ptr_ty], []>;
+
///===-------------------------- Other Intrinsics --------------------------===//
//
// TODO: We should introduce a new memory kind fo traps (and other side effects
diff --git a/llvm/lib/Transforms/Coroutines/CoroCleanup.cpp b/llvm/lib/Transforms/Coroutines/CoroCleanup.cpp
index 3e3825fcd50e2..71229eae5cb47 100644
--- a/llvm/lib/Transforms/Coroutines/CoroCleanup.cpp
+++ b/llvm/lib/Transforms/Coroutines/CoroCleanup.cpp
@@ -8,10 +8,11 @@
#include "llvm/Transforms/Coroutines/CoroCleanup.h"
#include "CoroInternal.h"
+#include "llvm/IR/Function.h"
#include "llvm/IR/IRBuilder.h"
#include "llvm/IR/InstIterator.h"
+#include "llvm/IR/Intrinsics.h"
#include "llvm/IR/PassManager.h"
-#include "llvm/IR/Function.h"
#include "llvm/Transforms/Scalar/SimplifyCFG.h"
using namespace llvm;
@@ -80,7 +81,7 @@ bool Lowerer::lower(Function &F) {
} else
continue;
break;
- case Intrinsic::coro_async_size_replace:
+ case Intrinsic::coro_async_size_replace: {
auto *Target = cast<ConstantStruct>(
cast<GlobalVariable>(II->getArgOperand(0)->stripPointerCasts())
->getInitializer());
@@ -98,6 +99,9 @@ bool Lowerer::lower(Function &F) {
Target->replaceAllUsesWith(NewFuncPtrStruct);
break;
}
+ case Intrinsic::coro_safe_elide:
+ break;
+ }
II->eraseFromParent();
Changed = true;
}
@@ -111,7 +115,8 @@ static bool declaresCoroCleanupIntrinsics(const Module &M) {
M, {"llvm.coro.alloc", "llvm.coro.begin", "llvm.coro.subfn.addr",
"llvm.coro.free", "llvm.coro.id", "llvm.coro.id.retcon",
"llvm.coro.id.async", "llvm.coro.id.retcon.once",
- "llvm.coro.async.size.replace", "llvm.coro.async.resume"});
+ "llvm.coro.async.size.replace", "llvm.coro.async.resume",
+ "llvm.coro.safe.elide"});
}
PreservedAnalyses CoroCleanupPass::run(Module &M,
diff --git a/llvm/lib/Transforms/Coroutines/CoroElide.cpp b/llvm/lib/Transforms/Coroutines/CoroElide.cpp
index 74b5ccb7b9b71..dd2f72410c931 100644
--- a/llvm/lib/Transforms/Coroutines/CoroElide.cpp
+++ b/llvm/lib/Transforms/Coroutines/CoroElide.cpp
@@ -7,12 +7,14 @@
//===----------------------------------------------------------------------===//
#include "llvm/Transforms/Coroutines/CoroElide.h"
+#include "CoroInstr.h"
#include "CoroInternal.h"
#include "llvm/ADT/DenseMap.h"
#include "llvm/ADT/Statistic.h"
#include "llvm/Analysis/AliasAnalysis.h"
#include "llvm/Analysis/InstructionSimplify.h"
#include "llvm/Analysis/OptimizationRemarkEmitter.h"
+#include "llvm/Analysis/PostDominators.h"
#include "llvm/IR/Dominators.h"
#include "llvm/IR/InstIterator.h"
#include "llvm/Support/ErrorHandling.h"
@@ -56,7 +58,8 @@ class FunctionElideInfo {
class CoroIdElider {
public:
CoroIdElider(CoroIdInst *CoroId, FunctionElideInfo &FEI, AAResults &AA,
- DominatorTree &DT, OptimizationRemarkEmitter &ORE);
+ DominatorTree &DT, PostDominatorTree &PDT,
+ OptimizationRemarkEmitter &ORE);
void elideHeapAllocations(uint64_t FrameSize, Align FrameAlign);
bool lifetimeEligibleForElide() const;
bool attemptElide();
@@ -68,6 +71,7 @@ class CoroIdElider {
FunctionElideInfo &FEI;
AAResults &AA;
DominatorTree &DT;
+ PostDominatorTree &PDT;
OptimizationRemarkEmitter &ORE;
SmallVector<CoroBeginInst *, 1> CoroBegins;
@@ -183,8 +187,9 @@ void FunctionElideInfo::collectPostSplitCoroIds() {
CoroIdElider::CoroIdElider(CoroIdInst *CoroId, FunctionElideInfo &FEI,
AAResults &AA, DominatorTree &DT,
+ PostDominatorTree &PDT,
OptimizationRemarkEmitter &ORE)
- : CoroId(CoroId), FEI(FEI), AA(AA), DT(DT), ORE(ORE) {
+ : CoroId(CoroId), FEI(FEI), AA(AA), DT(DT), PDT(PDT), ORE(ORE) {
// Collect all coro.begin and coro.allocs associated with this coro.id.
for (User *U : CoroId->users()) {
if (auto *CB = dyn_cast<CoroBeginInst>(U))
@@ -336,6 +341,41 @@ bool CoroIdElider::canCoroBeginEscape(
return false;
}
+// FIXME: This is not accounting for the stores to tasks whose handle is not
+// zero offset.
+static const StoreInst *getPostDominatingStoreToTask(const CoroBeginInst *CB,
+ PostDominatorTree &PDT) {
+ const StoreInst *OnlyStore = nullptr;
+
+ for (auto *U : CB->users()) {
+ auto *Store = dyn_cast<StoreInst>(U);
+ if (Store && Store->getValueOperand() == CB) {
+ if (OnlyStore) {
+ // Store must be unique. one coro begin getting stored to multiple
+ // stores is not accepted.
+ return nullptr;
+ }
+ OnlyStore = Store;
+ }
+ }
+
+ if (!OnlyStore || !PDT.dominates(OnlyStore, CB)) {
+ return nullptr;
+ }
+
+ return OnlyStore;
+}
+
+static bool isMarkedSafeElide(const llvm::Value *V) {
+ for (auto *U : V->users()) {
+ auto *II = dyn_cast<IntrinsicInst>(U);
+ if (II && (II->getIntrinsicID() == Intrinsic::coro_safe_elide)) {
+ return true;
+ }
+ }
+ return false;
+}
+
bool CoroIdElider::lifetimeEligibleForElide() const {
// If no CoroAllocs, we cannot suppress allocation, so elision is not
// possible.
@@ -364,6 +404,15 @@ bool CoroIdElider::lifetimeEligibleForElide() const {
// Filter out the coro.destroy that lie along exceptional paths.
for (const auto *CB : CoroBegins) {
+ // This might be too strong of a condition but should be very safe.
+ // If the CB is unconditionally stored into a "Task Like Object",
+ // and such object is "safe elide".
+ if (auto *MaybeStoreToTask = getPostDominatingStoreToTask(CB, PDT)) {
+ auto Dest = MaybeStoreToTask->getPointerOperand();
+ if (isMarkedSafeElide(Dest))
+ continue;
+ }
+
auto It = DestroyAddr.find(CB);
// FIXME: If we have not found any destroys for this coro.begin, we
@@ -476,11 +525,12 @@ PreservedAnalyses CoroElidePass::run(Function &F, FunctionAnalysisManager &AM) {
AAResults &AA = AM.getResult<AAManager>(F);
DominatorTree &DT = AM.getResult<DominatorTreeAnalysis>(F);
+ PostDominatorTree &PDT = AM.getResult<PostDominatorTreeAnalysis>(F);
auto &ORE = AM.getResult<OptimizationRemarkEmitterAnalysis>(F);
bool Changed = false;
for (auto *CII : FEI.getCoroIds()) {
- CoroIdElider CIE(CII, FEI, AA, DT, ORE);
+ CoroIdElider CIE(CII, FEI, AA, DT, PDT, ORE);
Changed |= CIE.attemptElide();
}
diff --git a/llvm/lib/Transforms/Coroutines/Coroutines.cpp b/llvm/lib/Transforms/Coroutines/Coroutines.cpp
index 1a92bc1636257..48c02e5406b75 100644
--- a/llvm/lib/Transforms/Coroutines/Coroutines.cpp
+++ b/llvm/lib/Transforms/Coroutines/Coroutines.cpp
@@ -86,6 +86,7 @@ static const char *const CoroIntrinsics[] = {
"llvm.coro.prepare.retcon",
"llvm.coro.promise",
"llvm.coro.resume",
+ "llvm.coro.safe.elide",
"llvm.coro.save",
"llvm.coro.size",
"llvm.coro.subfn.addr",
``````````
</details>
https://github.com/llvm/llvm-project/pull/94693
More information about the cfe-commits
mailing list