[LLVM][Coroutines] Transform "coro_elide_safe" calls to switch ABI coroutines to the noalloc variant
#99285
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
llvmbot
added
clang
|
@llvm/pr-subscribers-clang @llvm/pr-subscribers-llvm-ir Author: Yuxuan Chen (yuxuanchen1997) ChangesThis patch is episode three of the middle end implementation for the coroutine HALO improvement project published on discourse: https://discourse.llvm.org/t/language-extension-for-better-more-deterministic-halo-for-c-coroutines/80044 After we attribute the calls to some coroutines as "coro_must_elide" in the C++ FE and creating a This pass should be run after CoroSplit. For each node we process in CoroSplit, we look for its callers and replace the attributed ones in presplit coroutines to the noalloc one. The transformed Please note that we cannot safely transform such attributed calls in post-split coroutines due to memory lifetime reasons. The CoroSplit pass is responsible for creating the coroutine frame spills for all the allocas in the coroutine. Therefore it will be unsafe to create new allocas like this one in post-split coroutines. This happens relatively rarely because CGSCC performs the passes on the callees before the caller. However, if multiple coroutines coexist in one SCC, this situation does happen (and prevents us from having potentially unbound frame size due to recursion.) You can find episode 1: Clang FE of this patch series at #99282 Patch is 86.13 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/99285.diff 47 Files Affected:
diff --git a/clang/include/clang/AST/ExprCXX.h b/clang/include/clang/AST/ExprCXX.h
index c2feac525c1ea..0cf62aee41b66 100644
--- a/clang/include/clang/AST/ExprCXX.h
+++ b/clang/include/clang/AST/ExprCXX.h
@@ -5082,7 +5082,8 @@ class CoroutineSuspendExpr : public Expr {
enum SubExpr { Operand, Common, Ready, Suspend, Resume, Count };
Stmt *SubExprs[SubExpr::Count];
- OpaqueValueExpr *OpaqueValue = nullptr;
+ OpaqueValueExpr *CommonExprOpaqueValue = nullptr;
+ OpaqueValueExpr *InplaceCallOpaqueValue = nullptr;
public:
// These types correspond to the three C++ 'await_suspend' return variants
@@ -5090,10 +5091,10 @@ class CoroutineSuspendExpr : public Expr {
CoroutineSuspendExpr(StmtClass SC, SourceLocation KeywordLoc, Expr *Operand,
Expr *Common, Expr *Ready, Expr *Suspend, Expr *Resume,
- OpaqueValueExpr *OpaqueValue)
+ OpaqueValueExpr *CommonExprOpaqueValue)
: Expr(SC, Resume->getType(), Resume->getValueKind(),
Resume->getObjectKind()),
- KeywordLoc(KeywordLoc), OpaqueValue(OpaqueValue) {
+ KeywordLoc(KeywordLoc), CommonExprOpaqueValue(CommonExprOpaqueValue) {
SubExprs[SubExpr::Operand] = Operand;
SubExprs[SubExpr::Common] = Common;
SubExprs[SubExpr::Ready] = Ready;
@@ -5128,7 +5129,16 @@ class CoroutineSuspendExpr : public Expr {
}
/// getOpaqueValue - Return the opaque value placeholder.
- OpaqueValueExpr *getOpaqueValue() const { return OpaqueValue; }
+ OpaqueValueExpr *getCommonExprOpaqueValue() const {
+ return CommonExprOpaqueValue;
+ }
+
+ OpaqueValueExpr *getInplaceCallOpaqueValue() const {
+ return InplaceCallOpaqueValue;
+ }
+ void setInplaceCallOpaqueValue(OpaqueValueExpr *E) {
+ InplaceCallOpaqueValue = E;
+ }
Expr *getReadyExpr() const {
return static_cast<Expr*>(SubExprs[SubExpr::Ready]);
@@ -5194,9 +5204,9 @@ class CoawaitExpr : public CoroutineSuspendExpr {
public:
CoawaitExpr(SourceLocation CoawaitLoc, Expr *Operand, Expr *Common,
Expr *Ready, Expr *Suspend, Expr *Resume,
- OpaqueValueExpr *OpaqueValue, bool IsImplicit = false)
+ OpaqueValueExpr *CommonExprOpaqueValue, bool IsImplicit = false)
: CoroutineSuspendExpr(CoawaitExprClass, CoawaitLoc, Operand, Common,
- Ready, Suspend, Resume, OpaqueValue) {
+ Ready, Suspend, Resume, CommonExprOpaqueValue) {
CoawaitBits.IsImplicit = IsImplicit;
}
@@ -5275,9 +5285,9 @@ class CoyieldExpr : public CoroutineSuspendExpr {
public:
CoyieldExpr(SourceLocation CoyieldLoc, Expr *Operand, Expr *Common,
Expr *Ready, Expr *Suspend, Expr *Resume,
- OpaqueValueExpr *OpaqueValue)
+ OpaqueValueExpr *CommonExprOpaqueValue)
: CoroutineSuspendExpr(CoyieldExprClass, CoyieldLoc, Operand, Common,
- Ready, Suspend, Resume, OpaqueValue) {}
+ Ready, Suspend, Resume, CommonExprOpaqueValue) {}
CoyieldExpr(SourceLocation CoyieldLoc, QualType Ty, Expr *Operand,
Expr *Common)
: CoroutineSuspendExpr(CoyieldExprClass, CoyieldLoc, Ty, Operand,
diff --git a/clang/include/clang/Basic/Attr.td b/clang/include/clang/Basic/Attr.td
index 1293d0ddbc117..e482c9daf9fb3 100644
--- a/clang/include/clang/Basic/Attr.td
+++ b/clang/include/clang/Basic/Attr.td
@@ -1217,6 +1217,14 @@ def CoroDisableLifetimeBound : InheritableAttr {
let SimpleHandler = 1;
}
+def CoroInplaceTask : InheritableAttr {
+ let Spellings = [Clang<"coro_inplace_task">];
+ let Subjects = SubjectList<[CXXRecord]>;
+ let LangOpts = [CPlusPlus];
+ let Documentation = [CoroInplaceTaskDoc];
+ 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 09cf4f80bd999..21d59dedec578 100644
--- a/clang/include/clang/Basic/AttrDocs.td
+++ b/clang/include/clang/Basic/AttrDocs.td
@@ -8108,6 +8108,25 @@ but do not pass them to the underlying coroutine or pass them by value.
}];
}
+def CoroInplaceTaskDoc : Documentation {
+ let Category = DocCatDecl;
+ let Content = [{
+The ``[[clang::coro_inplace_task]]`` 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_inplace_task]]``.
+- 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/CGBlocks.cpp b/clang/lib/CodeGen/CGBlocks.cpp
index 066139b1c78c7..684fda7440731 100644
--- a/clang/lib/CodeGen/CGBlocks.cpp
+++ b/clang/lib/CodeGen/CGBlocks.cpp
@@ -1163,7 +1163,8 @@ llvm::Type *CodeGenModule::getGenericBlockLiteralType() {
}
RValue CodeGenFunction::EmitBlockCallExpr(const CallExpr *E,
- ReturnValueSlot ReturnValue) {
+ ReturnValueSlot ReturnValue,
+ llvm::CallBase **CallOrInvoke) {
const auto *BPT = E->getCallee()->getType()->castAs<BlockPointerType>();
llvm::Value *BlockPtr = EmitScalarExpr(E->getCallee());
llvm::Type *GenBlockTy = CGM.getGenericBlockLiteralType();
@@ -1220,7 +1221,7 @@ RValue CodeGenFunction::EmitBlockCallExpr(const CallExpr *E,
CGCallee Callee(CGCalleeInfo(), Func);
// And call the block.
- return EmitCall(FnInfo, Callee, ReturnValue, Args);
+ return EmitCall(FnInfo, Callee, ReturnValue, Args, CallOrInvoke);
}
Address CodeGenFunction::GetAddrOfBlockDecl(const VarDecl *variable) {
diff --git a/clang/lib/CodeGen/CGCUDARuntime.cpp b/clang/lib/CodeGen/CGCUDARuntime.cpp
index c14a9d3f2bbbc..1e1da1e2411a7 100644
--- a/clang/lib/CodeGen/CGCUDARuntime.cpp
+++ b/clang/lib/CodeGen/CGCUDARuntime.cpp
@@ -25,7 +25,8 @@ CGCUDARuntime::~CGCUDARuntime() {}
RValue CGCUDARuntime::EmitCUDAKernelCallExpr(CodeGenFunction &CGF,
const CUDAKernelCallExpr *E,
- ReturnValueSlot ReturnValue) {
+ ReturnValueSlot ReturnValue,
+ llvm::CallBase **CallOrInvoke) {
llvm::BasicBlock *ConfigOKBlock = CGF.createBasicBlock("kcall.configok");
llvm::BasicBlock *ContBlock = CGF.createBasicBlock("kcall.end");
@@ -35,7 +36,7 @@ RValue CGCUDARuntime::EmitCUDAKernelCallExpr(CodeGenFunction &CGF,
eval.begin(CGF);
CGF.EmitBlock(ConfigOKBlock);
- CGF.EmitSimpleCallExpr(E, ReturnValue);
+ CGF.EmitSimpleCallExpr(E, ReturnValue, CallOrInvoke);
CGF.EmitBranch(ContBlock);
CGF.EmitBlock(ContBlock);
diff --git a/clang/lib/CodeGen/CGCUDARuntime.h b/clang/lib/CodeGen/CGCUDARuntime.h
index 8030d632cc3d2..86f776004ee7c 100644
--- a/clang/lib/CodeGen/CGCUDARuntime.h
+++ b/clang/lib/CodeGen/CGCUDARuntime.h
@@ -21,6 +21,7 @@
#include "llvm/IR/GlobalValue.h"
namespace llvm {
+class CallBase;
class Function;
class GlobalVariable;
}
@@ -82,9 +83,10 @@ class CGCUDARuntime {
CGCUDARuntime(CodeGenModule &CGM) : CGM(CGM) {}
virtual ~CGCUDARuntime();
- virtual RValue EmitCUDAKernelCallExpr(CodeGenFunction &CGF,
- const CUDAKernelCallExpr *E,
- ReturnValueSlot ReturnValue);
+ virtual RValue
+ EmitCUDAKernelCallExpr(CodeGenFunction &CGF, const CUDAKernelCallExpr *E,
+ ReturnValueSlot ReturnValue,
+ llvm::CallBase **CallOrInvoke = nullptr);
/// Emits a kernel launch stub.
virtual void emitDeviceStub(CodeGenFunction &CGF, FunctionArgList &Args) = 0;
diff --git a/clang/lib/CodeGen/CGCXXABI.h b/clang/lib/CodeGen/CGCXXABI.h
index 7dcc539111996..687ff7fb84444 100644
--- a/clang/lib/CodeGen/CGCXXABI.h
+++ b/clang/lib/CodeGen/CGCXXABI.h
@@ -485,11 +485,11 @@ class CGCXXABI {
llvm::PointerUnion<const CXXDeleteExpr *, const CXXMemberCallExpr *>;
/// Emit the ABI-specific virtual destructor call.
- virtual llvm::Value *EmitVirtualDestructorCall(CodeGenFunction &CGF,
- const CXXDestructorDecl *Dtor,
- CXXDtorType DtorType,
- Address This,
- DeleteOrMemberCallExpr E) = 0;
+ virtual llvm::Value *
+ EmitVirtualDestructorCall(CodeGenFunction &CGF, const CXXDestructorDecl *Dtor,
+ CXXDtorType DtorType, Address This,
+ DeleteOrMemberCallExpr E,
+ llvm::CallBase **CallOrInvoke) = 0;
virtual void adjustCallArgsForDestructorThunk(CodeGenFunction &CGF,
GlobalDecl GD,
diff --git a/clang/lib/CodeGen/CGClass.cpp b/clang/lib/CodeGen/CGClass.cpp
index 0a595bb998d26..c56716fbd0590 100644
--- a/clang/lib/CodeGen/CGClass.cpp
+++ b/clang/lib/CodeGen/CGClass.cpp
@@ -2191,15 +2191,11 @@ static bool canEmitDelegateCallArgs(CodeGenFunction &CGF,
return true;
}
-void CodeGenFunction::EmitCXXConstructorCall(const CXXConstructorDecl *D,
- CXXCtorType Type,
- bool ForVirtualBase,
- bool Delegating,
- Address This,
- CallArgList &Args,
- AggValueSlot::Overlap_t Overlap,
- SourceLocation Loc,
- bool NewPointerIsChecked) {
+void CodeGenFunction::EmitCXXConstructorCall(
+ const CXXConstructorDecl *D, CXXCtorType Type, bool ForVirtualBase,
+ bool Delegating, Address This, CallArgList &Args,
+ AggValueSlot::Overlap_t Overlap, SourceLocation Loc,
+ bool NewPointerIsChecked, llvm::CallBase **CallOrInvoke) {
const CXXRecordDecl *ClassDecl = D->getParent();
if (!NewPointerIsChecked)
@@ -2247,7 +2243,7 @@ void CodeGenFunction::EmitCXXConstructorCall(const CXXConstructorDecl *D,
const CGFunctionInfo &Info = CGM.getTypes().arrangeCXXConstructorCall(
Args, D, Type, ExtraArgs.Prefix, ExtraArgs.Suffix, PassPrototypeArgs);
CGCallee Callee = CGCallee::forDirect(CalleePtr, GlobalDecl(D, Type));
- EmitCall(Info, Callee, ReturnValueSlot(), Args, nullptr, false, Loc);
+ EmitCall(Info, Callee, ReturnValueSlot(), Args, CallOrInvoke, false, Loc);
// Generate vtable assumptions if we're constructing a complete object
// with a vtable. We don't do this for base subobjects for two reasons:
diff --git a/clang/lib/CodeGen/CGCoroutine.cpp b/clang/lib/CodeGen/CGCoroutine.cpp
index a8a70186c2c5a..f04287b871029 100644
--- a/clang/lib/CodeGen/CGCoroutine.cpp
+++ b/clang/lib/CodeGen/CGCoroutine.cpp
@@ -12,9 +12,11 @@
#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/ScopeExit.h"
+#include "llvm/IR/Intrinsics.h"
using namespace clang;
using namespace CodeGen;
@@ -223,12 +225,22 @@ static LValueOrRValue emitSuspendExpression(CodeGenFunction &CGF, CGCoroData &Co
CoroutineSuspendExpr const &S,
AwaitKind Kind, AggValueSlot aggSlot,
bool ignoreResult, bool forLValue) {
- auto *E = S.getCommonExpr();
+ auto &Builder = CGF.Builder;
- auto CommonBinder =
- CodeGenFunction::OpaqueValueMappingData::bind(CGF, S.getOpaqueValue(), E);
- auto UnbindCommonOnExit =
- llvm::make_scope_exit([&] { CommonBinder.unbind(CGF); });
+ // If S.getInplaceCallOpaqueValue() is null, we don't have a nested opaque
+ // value for common expression.
+ std::optional<CodeGenFunction::OpaqueValueMapping> OperandMapping;
+ if (auto *CallOV = S.getInplaceCallOpaqueValue()) {
+ auto *CE = cast<CallExpr>(CallOV->getSourceExpr());
+ llvm::CallBase *CallOrInvoke = nullptr;
+ LValue CallResult = CGF.EmitCallExprLValue(CE, &CallOrInvoke);
+ if (CallOrInvoke)
+ CallOrInvoke->addFnAttr(llvm::Attribute::CoroMustElide);
+
+ OperandMapping.emplace(CGF, CallOV, CallResult);
+ }
+ CodeGenFunction::OpaqueValueMapping BindCommon(CGF,
+ S.getCommonExprOpaqueValue());
auto Prefix = buildSuspendPrefixStr(Coro, Kind);
BasicBlock *ReadyBlock = CGF.createBasicBlock(Prefix + Twine(".ready"));
@@ -241,7 +253,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});
@@ -256,7 +267,8 @@ static LValueOrRValue emitSuspendExpression(CodeGenFunction &CGF, CGCoroData &Co
SmallVector<llvm::Value *, 3> SuspendIntrinsicCallArgs;
SuspendIntrinsicCallArgs.push_back(
- CGF.getOrCreateOpaqueLValueMapping(S.getOpaqueValue()).getPointer(CGF));
+ CGF.getOrCreateOpaqueLValueMapping(S.getCommonExprOpaqueValue())
+ .getPointer(CGF));
SuspendIntrinsicCallArgs.push_back(CGF.CurCoro.Data->CoroBegin);
SuspendIntrinsicCallArgs.push_back(SuspendWrapper);
@@ -455,7 +467,7 @@ CodeGenFunction::generateAwaitSuspendWrapper(Twine const &CoroName,
Builder.CreateLoad(GetAddrOfLocalVar(&FrameDecl));
auto AwaiterBinder = CodeGenFunction::OpaqueValueMappingData::bind(
- *this, S.getOpaqueValue(), AwaiterLValue);
+ *this, S.getCommonExprOpaqueValue(), AwaiterLValue);
auto *SuspendRet = EmitScalarExpr(S.getSuspendExpr());
diff --git a/clang/lib/CodeGen/CGExpr.cpp b/clang/lib/CodeGen/CGExpr.cpp
index af1e1a25d1d8b..0ca2f5bbe823b 100644
--- a/clang/lib/CodeGen/CGExpr.cpp
+++ b/clang/lib/CodeGen/CGExpr.cpp
@@ -5437,16 +5437,17 @@ RValue CodeGenFunction::EmitRValueForField(LValue LV,
//===--------------------------------------------------------------------===//
RValue CodeGenFunction::EmitCallExpr(const CallExpr *E,
- ReturnValueSlot ReturnValue) {
+ ReturnValueSlot ReturnValue,
+ llvm::CallBase **CallOrInvoke) {
// Builtins never have block type.
if (E->getCallee()->getType()->isBlockPointerType())
- return EmitBlockCallExpr(E, ReturnValue);
+ return EmitBlockCallExpr(E, ReturnValue, CallOrInvoke);
if (const auto *CE = dyn_cast<CXXMemberCallExpr>(E))
- return EmitCXXMemberCallExpr(CE, ReturnValue);
+ return EmitCXXMemberCallExpr(CE, ReturnValue, CallOrInvoke);
if (const auto *CE = dyn_cast<CUDAKernelCallExpr>(E))
- return EmitCUDAKernelCallExpr(CE, ReturnValue);
+ return EmitCUDAKernelCallExpr(CE, ReturnValue, CallOrInvoke);
// A CXXOperatorCallExpr is created even for explicit object methods, but
// these should be treated like static function call.
@@ -5454,7 +5455,7 @@ RValue CodeGenFunction::EmitCallExpr(const CallExpr *E,
if (const auto *MD =
dyn_cast_if_present<CXXMethodDecl>(CE->getCalleeDecl());
MD && MD->isImplicitObjectMemberFunction())
- return EmitCXXOperatorMemberCallExpr(CE, MD, ReturnValue);
+ return EmitCXXOperatorMemberCallExpr(CE, MD, ReturnValue, CallOrInvoke);
CGCallee callee = EmitCallee(E->getCallee());
@@ -5467,14 +5468,17 @@ RValue CodeGenFunction::EmitCallExpr(const CallExpr *E,
return EmitCXXPseudoDestructorExpr(callee.getPseudoDestructorExpr());
}
- return EmitCall(E->getCallee()->getType(), callee, E, ReturnValue);
+ return EmitCall(E->getCallee()->getType(), callee, E, ReturnValue,
+ /*Chain=*/nullptr, CallOrInvoke);
}
/// Emit a CallExpr without considering whether it might be a subclass.
RValue CodeGenFunction::EmitSimpleCallExpr(const CallExpr *E,
- ReturnValueSlot ReturnValue) {
+ ReturnValueSlot ReturnValue,
+ llvm::CallBase **CallOrInvoke) {
CGCallee Callee = EmitCallee(E->getCallee());
- return EmitCall(E->getCallee()->getType(), Callee, E, ReturnValue);
+ return EmitCall(E->getCallee()->getType(), Callee, E, ReturnValue,
+ /*Chain=*/nullptr, CallOrInvoke);
}
// Detect the unusual situation where an inline version is shadowed by a
@@ -5678,8 +5682,9 @@ LValue CodeGenFunction::EmitBinaryOperatorLValue(const BinaryOperator *E) {
llvm_unreachable("bad evaluation kind");
}
-LValue CodeGenFunction::EmitCallExprLValue(const CallExpr *E) {
- RValue RV = EmitCallExpr(E);
+LValue CodeGenFunction::EmitCallExprLValue(const CallExpr *E,
+ llvm::CallBase **CallOrInvoke) {
+ RValue RV = EmitCallExpr(E, ReturnValueSlot(), CallOrInvoke);
if (!RV.isScalar())
return MakeAddrLValue(RV.getAggregateAddress(), E->getType(),
@@ -5802,9 +5807,11 @@ LValue CodeGenFunction::EmitStmtExprLValue(const StmtExpr *E) {
AlignmentSource::Decl);
}
-RValue CodeGenFunction::EmitCall(QualType CalleeType, const CGCallee &OrigCallee,
- const CallExpr *E, ReturnValueSlot ReturnValue,
- llvm::Value *Chain) {
+RValue CodeGenFunction::EmitCall(QualType CalleeType,
+ const CGCallee &OrigCallee, const CallExpr *E,
+ ReturnValueSlot ReturnValue,
+ llvm::Value *Chain,
+ llvm::CallBase **CallOrInvoke) {
// Get the actual function type. The callee type will always be a pointer to
// function type or a block pointer type.
assert(CalleeType->isFunctionPointerType() &&
@@ -6015,8 +6022,8 @@ RValue CodeGenFunction::EmitCall(QualType CalleeType, const CGCallee &OrigCallee
Address(Handle, Handle->getType(), CGM.getPointerAlign()));
Callee.setFunctionPointer(Stub);
}
- llvm::CallBase *CallOrInvoke = nullptr;
- RValue Call = EmitCall(FnInfo, Callee, ReturnValue, Args, &CallOrInvoke,
+ llvm::CallBase *LocalCallOrInvoke = nullptr;
+ RValue Call = EmitCall(FnInfo, Callee, ReturnValue, Args, &LocalCallOrInvoke,
E == MustTailCall, E->getExprLoc());
// Generate function declaration DISuprogram in order to be used
@@ -6025,11 +6032,13 @@ RValue CodeGenFunction::EmitCall(QualType CalleeType, const CGCallee &OrigCallee
if (auto *CalleeDecl = dyn_cast_or_null<FunctionDecl>(TargetDecl)) {
FunctionArgList Args;
QualType ResTy = BuildFunctionArgList(CalleeDecl, Args);
- DI->EmitFuncDeclForCallSite(CallOrInvoke,
+ DI->EmitFuncDeclForCallSite(LocalCallOrInvoke,
DI->getFunctionType(CalleeDecl, ResTy, Args),
CalleeDecl);
}
}
+ if (CallOrInvoke)
+ *CallOrInvoke = LocalCallOrInvoke;
return Call;
}
diff --git a/clang/lib/CodeGen/CGExprCXX.cpp b/clang/lib/CodeGen/CGExprCXX.cpp
index 8eb6ab7381acb..1214bb054fb8d 100644
--- a/clang/lib/CodeGen/CGExprCXX.cpp
+++ b/clang/lib/CodeGen/CGExprCXX.cpp
@@ -84,23 +84,24 @@ commonEmitCXXMemberOrOperatorCall(CodeGenFunction &CGF, GlobalDecl GD,
RValue CodeGenFunction::EmitCXXMemberOrOperatorCall(
const CXXMethodDecl *MD, const CGCallee &Callee,
- Retur...
[truncated]
|
|
@llvm/pr-subscribers-llvm-transforms Author: Yuxuan Chen (yuxuanchen1997) ChangesThis patch is episode three of the middle end implementation for the coroutine HALO improvement project published on discourse: https://discourse.llvm.org/t/language-extension-for-better-more-deterministic-halo-for-c-coroutines/80044 After we attribute the calls to some coroutines as "coro_must_elide" in the C++ FE and creating a This pass should be run after CoroSplit. For each node we process in CoroSplit, we look for its callers and replace the attributed ones in presplit coroutines to the noalloc one. The transformed Please note that we cannot safely transform such attributed calls in post-split coroutines due to memory lifetime reasons. The CoroSplit pass is responsible for creating the coroutine frame spills for all the allocas in the coroutine. Therefore it will be unsafe to create new allocas like this one in post-split coroutines. This happens relatively rarely because CGSCC performs the passes on the callees before the caller. However, if multiple coroutines coexist in one SCC, this situation does happen (and prevents us from having potentially unbound frame size due to recursion.) You can find episode 1: Clang FE of this patch series at #99282 Patch is 86.13 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/99285.diff 47 Files Affected:
diff --git a/clang/include/clang/AST/ExprCXX.h b/clang/include/clang/AST/ExprCXX.h
index c2feac525c1ea..0cf62aee41b66 100644
--- a/clang/include/clang/AST/ExprCXX.h
+++ b/clang/include/clang/AST/ExprCXX.h
@@ -5082,7 +5082,8 @@ class CoroutineSuspendExpr : public Expr {
enum SubExpr { Operand, Common, Ready, Suspend, Resume, Count };
Stmt *SubExprs[SubExpr::Count];
- OpaqueValueExpr *OpaqueValue = nullptr;
+ OpaqueValueExpr *CommonExprOpaqueValue = nullptr;
+ OpaqueValueExpr *InplaceCallOpaqueValue = nullptr;
public:
// These types correspond to the three C++ 'await_suspend' return variants
@@ -5090,10 +5091,10 @@ class CoroutineSuspendExpr : public Expr {
CoroutineSuspendExpr(StmtClass SC, SourceLocation KeywordLoc, Expr *Operand,
Expr *Common, Expr *Ready, Expr *Suspend, Expr *Resume,
- OpaqueValueExpr *OpaqueValue)
+ OpaqueValueExpr *CommonExprOpaqueValue)
: Expr(SC, Resume->getType(), Resume->getValueKind(),
Resume->getObjectKind()),
- KeywordLoc(KeywordLoc), OpaqueValue(OpaqueValue) {
+ KeywordLoc(KeywordLoc), CommonExprOpaqueValue(CommonExprOpaqueValue) {
SubExprs[SubExpr::Operand] = Operand;
SubExprs[SubExpr::Common] = Common;
SubExprs[SubExpr::Ready] = Ready;
@@ -5128,7 +5129,16 @@ class CoroutineSuspendExpr : public Expr {
}
/// getOpaqueValue - Return the opaque value placeholder.
- OpaqueValueExpr *getOpaqueValue() const { return OpaqueValue; }
+ OpaqueValueExpr *getCommonExprOpaqueValue() const {
+ return CommonExprOpaqueValue;
+ }
+
+ OpaqueValueExpr *getInplaceCallOpaqueValue() const {
+ return InplaceCallOpaqueValue;
+ }
+ void setInplaceCallOpaqueValue(OpaqueValueExpr *E) {
+ InplaceCallOpaqueValue = E;
+ }
Expr *getReadyExpr() const {
return static_cast<Expr*>(SubExprs[SubExpr::Ready]);
@@ -5194,9 +5204,9 @@ class CoawaitExpr : public CoroutineSuspendExpr {
public:
CoawaitExpr(SourceLocation CoawaitLoc, Expr *Operand, Expr *Common,
Expr *Ready, Expr *Suspend, Expr *Resume,
- OpaqueValueExpr *OpaqueValue, bool IsImplicit = false)
+ OpaqueValueExpr *CommonExprOpaqueValue, bool IsImplicit = false)
: CoroutineSuspendExpr(CoawaitExprClass, CoawaitLoc, Operand, Common,
- Ready, Suspend, Resume, OpaqueValue) {
+ Ready, Suspend, Resume, CommonExprOpaqueValue) {
CoawaitBits.IsImplicit = IsImplicit;
}
@@ -5275,9 +5285,9 @@ class CoyieldExpr : public CoroutineSuspendExpr {
public:
CoyieldExpr(SourceLocation CoyieldLoc, Expr *Operand, Expr *Common,
Expr *Ready, Expr *Suspend, Expr *Resume,
- OpaqueValueExpr *OpaqueValue)
+ OpaqueValueExpr *CommonExprOpaqueValue)
: CoroutineSuspendExpr(CoyieldExprClass, CoyieldLoc, Operand, Common,
- Ready, Suspend, Resume, OpaqueValue) {}
+ Ready, Suspend, Resume, CommonExprOpaqueValue) {}
CoyieldExpr(SourceLocation CoyieldLoc, QualType Ty, Expr *Operand,
Expr *Common)
: CoroutineSuspendExpr(CoyieldExprClass, CoyieldLoc, Ty, Operand,
diff --git a/clang/include/clang/Basic/Attr.td b/clang/include/clang/Basic/Attr.td
index 1293d0ddbc117..e482c9daf9fb3 100644
--- a/clang/include/clang/Basic/Attr.td
+++ b/clang/include/clang/Basic/Attr.td
@@ -1217,6 +1217,14 @@ def CoroDisableLifetimeBound : InheritableAttr {
let SimpleHandler = 1;
}
+def CoroInplaceTask : InheritableAttr {
+ let Spellings = [Clang<"coro_inplace_task">];
+ let Subjects = SubjectList<[CXXRecord]>;
+ let LangOpts = [CPlusPlus];
+ let Documentation = [CoroInplaceTaskDoc];
+ 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 09cf4f80bd999..21d59dedec578 100644
--- a/clang/include/clang/Basic/AttrDocs.td
+++ b/clang/include/clang/Basic/AttrDocs.td
@@ -8108,6 +8108,25 @@ but do not pass them to the underlying coroutine or pass them by value.
}];
}
+def CoroInplaceTaskDoc : Documentation {
+ let Category = DocCatDecl;
+ let Content = [{
+The ``[[clang::coro_inplace_task]]`` 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_inplace_task]]``.
+- 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/CGBlocks.cpp b/clang/lib/CodeGen/CGBlocks.cpp
index 066139b1c78c7..684fda7440731 100644
--- a/clang/lib/CodeGen/CGBlocks.cpp
+++ b/clang/lib/CodeGen/CGBlocks.cpp
@@ -1163,7 +1163,8 @@ llvm::Type *CodeGenModule::getGenericBlockLiteralType() {
}
RValue CodeGenFunction::EmitBlockCallExpr(const CallExpr *E,
- ReturnValueSlot ReturnValue) {
+ ReturnValueSlot ReturnValue,
+ llvm::CallBase **CallOrInvoke) {
const auto *BPT = E->getCallee()->getType()->castAs<BlockPointerType>();
llvm::Value *BlockPtr = EmitScalarExpr(E->getCallee());
llvm::Type *GenBlockTy = CGM.getGenericBlockLiteralType();
@@ -1220,7 +1221,7 @@ RValue CodeGenFunction::EmitBlockCallExpr(const CallExpr *E,
CGCallee Callee(CGCalleeInfo(), Func);
// And call the block.
- return EmitCall(FnInfo, Callee, ReturnValue, Args);
+ return EmitCall(FnInfo, Callee, ReturnValue, Args, CallOrInvoke);
}
Address CodeGenFunction::GetAddrOfBlockDecl(const VarDecl *variable) {
diff --git a/clang/lib/CodeGen/CGCUDARuntime.cpp b/clang/lib/CodeGen/CGCUDARuntime.cpp
index c14a9d3f2bbbc..1e1da1e2411a7 100644
--- a/clang/lib/CodeGen/CGCUDARuntime.cpp
+++ b/clang/lib/CodeGen/CGCUDARuntime.cpp
@@ -25,7 +25,8 @@ CGCUDARuntime::~CGCUDARuntime() {}
RValue CGCUDARuntime::EmitCUDAKernelCallExpr(CodeGenFunction &CGF,
const CUDAKernelCallExpr *E,
- ReturnValueSlot ReturnValue) {
+ ReturnValueSlot ReturnValue,
+ llvm::CallBase **CallOrInvoke) {
llvm::BasicBlock *ConfigOKBlock = CGF.createBasicBlock("kcall.configok");
llvm::BasicBlock *ContBlock = CGF.createBasicBlock("kcall.end");
@@ -35,7 +36,7 @@ RValue CGCUDARuntime::EmitCUDAKernelCallExpr(CodeGenFunction &CGF,
eval.begin(CGF);
CGF.EmitBlock(ConfigOKBlock);
- CGF.EmitSimpleCallExpr(E, ReturnValue);
+ CGF.EmitSimpleCallExpr(E, ReturnValue, CallOrInvoke);
CGF.EmitBranch(ContBlock);
CGF.EmitBlock(ContBlock);
diff --git a/clang/lib/CodeGen/CGCUDARuntime.h b/clang/lib/CodeGen/CGCUDARuntime.h
index 8030d632cc3d2..86f776004ee7c 100644
--- a/clang/lib/CodeGen/CGCUDARuntime.h
+++ b/clang/lib/CodeGen/CGCUDARuntime.h
@@ -21,6 +21,7 @@
#include "llvm/IR/GlobalValue.h"
namespace llvm {
+class CallBase;
class Function;
class GlobalVariable;
}
@@ -82,9 +83,10 @@ class CGCUDARuntime {
CGCUDARuntime(CodeGenModule &CGM) : CGM(CGM) {}
virtual ~CGCUDARuntime();
- virtual RValue EmitCUDAKernelCallExpr(CodeGenFunction &CGF,
- const CUDAKernelCallExpr *E,
- ReturnValueSlot ReturnValue);
+ virtual RValue
+ EmitCUDAKernelCallExpr(CodeGenFunction &CGF, const CUDAKernelCallExpr *E,
+ ReturnValueSlot ReturnValue,
+ llvm::CallBase **CallOrInvoke = nullptr);
/// Emits a kernel launch stub.
virtual void emitDeviceStub(CodeGenFunction &CGF, FunctionArgList &Args) = 0;
diff --git a/clang/lib/CodeGen/CGCXXABI.h b/clang/lib/CodeGen/CGCXXABI.h
index 7dcc539111996..687ff7fb84444 100644
--- a/clang/lib/CodeGen/CGCXXABI.h
+++ b/clang/lib/CodeGen/CGCXXABI.h
@@ -485,11 +485,11 @@ class CGCXXABI {
llvm::PointerUnion<const CXXDeleteExpr *, const CXXMemberCallExpr *>;
/// Emit the ABI-specific virtual destructor call.
- virtual llvm::Value *EmitVirtualDestructorCall(CodeGenFunction &CGF,
- const CXXDestructorDecl *Dtor,
- CXXDtorType DtorType,
- Address This,
- DeleteOrMemberCallExpr E) = 0;
+ virtual llvm::Value *
+ EmitVirtualDestructorCall(CodeGenFunction &CGF, const CXXDestructorDecl *Dtor,
+ CXXDtorType DtorType, Address This,
+ DeleteOrMemberCallExpr E,
+ llvm::CallBase **CallOrInvoke) = 0;
virtual void adjustCallArgsForDestructorThunk(CodeGenFunction &CGF,
GlobalDecl GD,
diff --git a/clang/lib/CodeGen/CGClass.cpp b/clang/lib/CodeGen/CGClass.cpp
index 0a595bb998d26..c56716fbd0590 100644
--- a/clang/lib/CodeGen/CGClass.cpp
+++ b/clang/lib/CodeGen/CGClass.cpp
@@ -2191,15 +2191,11 @@ static bool canEmitDelegateCallArgs(CodeGenFunction &CGF,
return true;
}
-void CodeGenFunction::EmitCXXConstructorCall(const CXXConstructorDecl *D,
- CXXCtorType Type,
- bool ForVirtualBase,
- bool Delegating,
- Address This,
- CallArgList &Args,
- AggValueSlot::Overlap_t Overlap,
- SourceLocation Loc,
- bool NewPointerIsChecked) {
+void CodeGenFunction::EmitCXXConstructorCall(
+ const CXXConstructorDecl *D, CXXCtorType Type, bool ForVirtualBase,
+ bool Delegating, Address This, CallArgList &Args,
+ AggValueSlot::Overlap_t Overlap, SourceLocation Loc,
+ bool NewPointerIsChecked, llvm::CallBase **CallOrInvoke) {
const CXXRecordDecl *ClassDecl = D->getParent();
if (!NewPointerIsChecked)
@@ -2247,7 +2243,7 @@ void CodeGenFunction::EmitCXXConstructorCall(const CXXConstructorDecl *D,
const CGFunctionInfo &Info = CGM.getTypes().arrangeCXXConstructorCall(
Args, D, Type, ExtraArgs.Prefix, ExtraArgs.Suffix, PassPrototypeArgs);
CGCallee Callee = CGCallee::forDirect(CalleePtr, GlobalDecl(D, Type));
- EmitCall(Info, Callee, ReturnValueSlot(), Args, nullptr, false, Loc);
+ EmitCall(Info, Callee, ReturnValueSlot(), Args, CallOrInvoke, false, Loc);
// Generate vtable assumptions if we're constructing a complete object
// with a vtable. We don't do this for base subobjects for two reasons:
diff --git a/clang/lib/CodeGen/CGCoroutine.cpp b/clang/lib/CodeGen/CGCoroutine.cpp
index a8a70186c2c5a..f04287b871029 100644
--- a/clang/lib/CodeGen/CGCoroutine.cpp
+++ b/clang/lib/CodeGen/CGCoroutine.cpp
@@ -12,9 +12,11 @@
#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/ScopeExit.h"
+#include "llvm/IR/Intrinsics.h"
using namespace clang;
using namespace CodeGen;
@@ -223,12 +225,22 @@ static LValueOrRValue emitSuspendExpression(CodeGenFunction &CGF, CGCoroData &Co
CoroutineSuspendExpr const &S,
AwaitKind Kind, AggValueSlot aggSlot,
bool ignoreResult, bool forLValue) {
- auto *E = S.getCommonExpr();
+ auto &Builder = CGF.Builder;
- auto CommonBinder =
- CodeGenFunction::OpaqueValueMappingData::bind(CGF, S.getOpaqueValue(), E);
- auto UnbindCommonOnExit =
- llvm::make_scope_exit([&] { CommonBinder.unbind(CGF); });
+ // If S.getInplaceCallOpaqueValue() is null, we don't have a nested opaque
+ // value for common expression.
+ std::optional<CodeGenFunction::OpaqueValueMapping> OperandMapping;
+ if (auto *CallOV = S.getInplaceCallOpaqueValue()) {
+ auto *CE = cast<CallExpr>(CallOV->getSourceExpr());
+ llvm::CallBase *CallOrInvoke = nullptr;
+ LValue CallResult = CGF.EmitCallExprLValue(CE, &CallOrInvoke);
+ if (CallOrInvoke)
+ CallOrInvoke->addFnAttr(llvm::Attribute::CoroMustElide);
+
+ OperandMapping.emplace(CGF, CallOV, CallResult);
+ }
+ CodeGenFunction::OpaqueValueMapping BindCommon(CGF,
+ S.getCommonExprOpaqueValue());
auto Prefix = buildSuspendPrefixStr(Coro, Kind);
BasicBlock *ReadyBlock = CGF.createBasicBlock(Prefix + Twine(".ready"));
@@ -241,7 +253,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});
@@ -256,7 +267,8 @@ static LValueOrRValue emitSuspendExpression(CodeGenFunction &CGF, CGCoroData &Co
SmallVector<llvm::Value *, 3> SuspendIntrinsicCallArgs;
SuspendIntrinsicCallArgs.push_back(
- CGF.getOrCreateOpaqueLValueMapping(S.getOpaqueValue()).getPointer(CGF));
+ CGF.getOrCreateOpaqueLValueMapping(S.getCommonExprOpaqueValue())
+ .getPointer(CGF));
SuspendIntrinsicCallArgs.push_back(CGF.CurCoro.Data->CoroBegin);
SuspendIntrinsicCallArgs.push_back(SuspendWrapper);
@@ -455,7 +467,7 @@ CodeGenFunction::generateAwaitSuspendWrapper(Twine const &CoroName,
Builder.CreateLoad(GetAddrOfLocalVar(&FrameDecl));
auto AwaiterBinder = CodeGenFunction::OpaqueValueMappingData::bind(
- *this, S.getOpaqueValue(), AwaiterLValue);
+ *this, S.getCommonExprOpaqueValue(), AwaiterLValue);
auto *SuspendRet = EmitScalarExpr(S.getSuspendExpr());
diff --git a/clang/lib/CodeGen/CGExpr.cpp b/clang/lib/CodeGen/CGExpr.cpp
index af1e1a25d1d8b..0ca2f5bbe823b 100644
--- a/clang/lib/CodeGen/CGExpr.cpp
+++ b/clang/lib/CodeGen/CGExpr.cpp
@@ -5437,16 +5437,17 @@ RValue CodeGenFunction::EmitRValueForField(LValue LV,
//===--------------------------------------------------------------------===//
RValue CodeGenFunction::EmitCallExpr(const CallExpr *E,
- ReturnValueSlot ReturnValue) {
+ ReturnValueSlot ReturnValue,
+ llvm::CallBase **CallOrInvoke) {
// Builtins never have block type.
if (E->getCallee()->getType()->isBlockPointerType())
- return EmitBlockCallExpr(E, ReturnValue);
+ return EmitBlockCallExpr(E, ReturnValue, CallOrInvoke);
if (const auto *CE = dyn_cast<CXXMemberCallExpr>(E))
- return EmitCXXMemberCallExpr(CE, ReturnValue);
+ return EmitCXXMemberCallExpr(CE, ReturnValue, CallOrInvoke);
if (const auto *CE = dyn_cast<CUDAKernelCallExpr>(E))
- return EmitCUDAKernelCallExpr(CE, ReturnValue);
+ return EmitCUDAKernelCallExpr(CE, ReturnValue, CallOrInvoke);
// A CXXOperatorCallExpr is created even for explicit object methods, but
// these should be treated like static function call.
@@ -5454,7 +5455,7 @@ RValue CodeGenFunction::EmitCallExpr(const CallExpr *E,
if (const auto *MD =
dyn_cast_if_present<CXXMethodDecl>(CE->getCalleeDecl());
MD && MD->isImplicitObjectMemberFunction())
- return EmitCXXOperatorMemberCallExpr(CE, MD, ReturnValue);
+ return EmitCXXOperatorMemberCallExpr(CE, MD, ReturnValue, CallOrInvoke);
CGCallee callee = EmitCallee(E->getCallee());
@@ -5467,14 +5468,17 @@ RValue CodeGenFunction::EmitCallExpr(const CallExpr *E,
return EmitCXXPseudoDestructorExpr(callee.getPseudoDestructorExpr());
}
- return EmitCall(E->getCallee()->getType(), callee, E, ReturnValue);
+ return EmitCall(E->getCallee()->getType(), callee, E, ReturnValue,
+ /*Chain=*/nullptr, CallOrInvoke);
}
/// Emit a CallExpr without considering whether it might be a subclass.
RValue CodeGenFunction::EmitSimpleCallExpr(const CallExpr *E,
- ReturnValueSlot ReturnValue) {
+ ReturnValueSlot ReturnValue,
+ llvm::CallBase **CallOrInvoke) {
CGCallee Callee = EmitCallee(E->getCallee());
- return EmitCall(E->getCallee()->getType(), Callee, E, ReturnValue);
+ return EmitCall(E->getCallee()->getType(), Callee, E, ReturnValue,
+ /*Chain=*/nullptr, CallOrInvoke);
}
// Detect the unusual situation where an inline version is shadowed by a
@@ -5678,8 +5682,9 @@ LValue CodeGenFunction::EmitBinaryOperatorLValue(const BinaryOperator *E) {
llvm_unreachable("bad evaluation kind");
}
-LValue CodeGenFunction::EmitCallExprLValue(const CallExpr *E) {
- RValue RV = EmitCallExpr(E);
+LValue CodeGenFunction::EmitCallExprLValue(const CallExpr *E,
+ llvm::CallBase **CallOrInvoke) {
+ RValue RV = EmitCallExpr(E, ReturnValueSlot(), CallOrInvoke);
if (!RV.isScalar())
return MakeAddrLValue(RV.getAggregateAddress(), E->getType(),
@@ -5802,9 +5807,11 @@ LValue CodeGenFunction::EmitStmtExprLValue(const StmtExpr *E) {
AlignmentSource::Decl);
}
-RValue CodeGenFunction::EmitCall(QualType CalleeType, const CGCallee &OrigCallee,
- const CallExpr *E, ReturnValueSlot ReturnValue,
- llvm::Value *Chain) {
+RValue CodeGenFunction::EmitCall(QualType CalleeType,
+ const CGCallee &OrigCallee, const CallExpr *E,
+ ReturnValueSlot ReturnValue,
+ llvm::Value *Chain,
+ llvm::CallBase **CallOrInvoke) {
// Get the actual function type. The callee type will always be a pointer to
// function type or a block pointer type.
assert(CalleeType->isFunctionPointerType() &&
@@ -6015,8 +6022,8 @@ RValue CodeGenFunction::EmitCall(QualType CalleeType, const CGCallee &OrigCallee
Address(Handle, Handle->getType(), CGM.getPointerAlign()));
Callee.setFunctionPointer(Stub);
}
- llvm::CallBase *CallOrInvoke = nullptr;
- RValue Call = EmitCall(FnInfo, Callee, ReturnValue, Args, &CallOrInvoke,
+ llvm::CallBase *LocalCallOrInvoke = nullptr;
+ RValue Call = EmitCall(FnInfo, Callee, ReturnValue, Args, &LocalCallOrInvoke,
E == MustTailCall, E->getExprLoc());
// Generate function declaration DISuprogram in order to be used
@@ -6025,11 +6032,13 @@ RValue CodeGenFunction::EmitCall(QualType CalleeType, const CGCallee &OrigCallee
if (auto *CalleeDecl = dyn_cast_or_null<FunctionDecl>(TargetDecl)) {
FunctionArgList Args;
QualType ResTy = BuildFunctionArgList(CalleeDecl, Args);
- DI->EmitFuncDeclForCallSite(CallOrInvoke,
+ DI->EmitFuncDeclForCallSite(LocalCallOrInvoke,
DI->getFunctionType(CalleeDecl, ResTy, Args),
CalleeDecl);
}
}
+ if (CallOrInvoke)
+ *CallOrInvoke = LocalCallOrInvoke;
return Call;
}
diff --git a/clang/lib/CodeGen/CGExprCXX.cpp b/clang/lib/CodeGen/CGExprCXX.cpp
index 8eb6ab7381acb..1214bb054fb8d 100644
--- a/clang/lib/CodeGen/CGExprCXX.cpp
+++ b/clang/lib/CodeGen/CGExprCXX.cpp
@@ -84,23 +84,24 @@ commonEmitCXXMemberOrOperatorCall(CodeGenFunction &CGF, GlobalDecl GD,
RValue CodeGenFunction::EmitCXXMemberOrOperatorCall(
const CXXMethodDecl *MD, const CGCallee &Callee,
- Retur...
[truncated]
|
yuxuanchen1997
changed the title
noalloc variantnoalloc variant
yuxuanchen1997
changed the base branch from
main
to
users/yuxuanchen1997/coro-split-noalloc
yuxuanchen1997
force-pushed
the
users/yuxuanchen1997/coro-split-noalloc
branch
from
19bd17b to
7c14a80
Compare
yuxuanchen1997
force-pushed
the
users/yuxuanchen1997/coro-transform-attr-must-elide
branch
from
c8475c0 to
08760fa
Compare
| MainCGPipeline.addPass(CoroSplitPass(Level != OptimizationLevel::O0)); | ||
| MainCGPipeline.addPass(CoroAnnotationElidePass()); |
There was a problem hiding this comment.
should this also be added to buildModuleInlinerPipeline?
There was a problem hiding this comment.
I applied this suggestion. However, looking at the buildModuleInlinerPipeline. It looks like it uses an adapter that runs single CGSCC pass on every function in the module. This won't work well for CoroAnnotationElidePass actually. It depends on the callee to be split, but not the caller.
Thinking about this, this is actually the same condition as the old CoroElidePass. Maybe the right thing to do here is to make this pass a function pass instead and use createCGSCCToFunctionPassAdaptor. What do you think?
yuxuanchen1997
force-pushed
the
users/yuxuanchen1997/coro-split-noalloc
branch
from
7c14a80 to
d42ce99
Compare
yuxuanchen1997
force-pushed
the
users/yuxuanchen1997/coro-transform-attr-must-elide
branch
from
08760fa to
7b89a7c
Compare
yuxuanchen1997
requested review from
JDevlieghere,
daniel-grumberg,
cyndyishida,
aaupov and
maksfb
as code owners
yuxuanchen1997
force-pushed
the
users/yuxuanchen1997/coro-transform-attr-must-elide
branch
from
0b3fc71 to
d6f2e78
Compare
yuxuanchen1997
force-pushed
the
users/yuxuanchen1997/coro-split-noalloc
branch
from
4084de7 to
e2a6027
Compare
|
✅ With the latest revision this PR passed the C/C++ code formatter. |
You can test this locally with the following command:git-clang-format --diff e2a6027dd2af62f4fbfa92795873f0489fd35cfd d6f2e78230c0907db95568e5b920d574ce6b4758 --extensions h,cpp -- llvm/include/llvm/Transforms/Coroutines/CoroAnnotationElide.h llvm/lib/Transforms/Coroutines/CoroAnnotationElide.cpp llvm/lib/Passes/PassBuilder.cpp llvm/lib/Passes/PassBuilderPipelines.cppView the diff from clang-format here.diff --git a/llvm/lib/Transforms/Coroutines/CoroAnnotationElide.cpp b/llvm/lib/Transforms/Coroutines/CoroAnnotationElide.cpp
index e7c7e01f9c..28953f2137 100644
--- a/llvm/lib/Transforms/Coroutines/CoroAnnotationElide.cpp
+++ b/llvm/lib/Transforms/Coroutines/CoroAnnotationElide.cpp
@@ -143,9 +143,9 @@ PreservedAnalyses CoroAnnotationElidePass::run(LazyCallGraph::SCC &C,
<< "' elided in '" << ore::NV("caller", Caller->getName());
});
Changed = true;
- updateCGAndAnalysisManagerForCGSCCPass(CG, *CallerC, *CallerN, AM, UR, FAM);
+ updateCGAndAnalysisManagerForCGSCCPass(CG, *CallerC, *CallerN, AM, UR,
+ FAM);
}
-
}
}
return Changed ? PreservedAnalyses::none() : PreservedAnalyses::all();
|
yuxuanchen1997
force-pushed
the
users/yuxuanchen1997/coro-transform-attr-must-elide
branch
2 times, most recently
from
68a410d to
5b18641
Compare
…ramp functions during CoroSplit
…routines to the `noalloc` variant
yuxuanchen1997
force-pushed
the
users/yuxuanchen1997/coro-transform-attr-must-elide
branch
from
5b18641 to
201bca0
Compare
yuxuanchen1997
force-pushed
the
users/yuxuanchen1997/coro-split-noalloc
branch
from
e2a6027 to
0c712a2
Compare
yuxuanchen1997
added a commit
that referenced
this pull request
Sep 9, 2024
…await_elidable]] (#99282) This patch is the frontend implementation of the coroutine elide improvement project detailed in this discourse post: https://discourse.llvm.org/t/language-extension-for-better-more-deterministic-halo-for-c-coroutines/80044 This patch proposes a C++ struct/class attribute `[[clang::coro_await_elidable]]`. This notion of await elidable task gives developers and library authors a certainty that coroutine heap elision happens in a predictable way. Originally, after we lower a coroutine to LLVM IR, CoroElide is responsible for analysis of whether an elision can happen. Take this as an example: ``` Task foo(); Task bar() { co_await foo(); } ``` For CoroElide to happen, the ramp function of `foo` must be inlined into `bar`. This inlining happens after `foo` has been split but `bar` is usually still a presplit coroutine. If `foo` is indeed a coroutine, the inlined `coro.id` intrinsics of `foo` is visible within `bar`. CoroElide then runs an analysis to figure out whether the SSA value of `coro.begin()` of `foo` gets destroyed before `bar` terminates. `Task` types are rarely simple enough for the destroy logic of the task to reference the SSA value from `coro.begin()` directly. Hence, the pass is very ineffective for even the most trivial C++ Task types. Improving CoroElide by implementing more powerful analyses is possible, however it doesn't give us the predictability when we expect elision to happen. The approach we want to take with this language extension generally originates from the philosophy that library implementations of `Task` types has the control over the structured concurrency guarantees we demand for elision to happen. That is, the lifetime for the callee's frame is shorter to that of the caller. The ``[[clang::coro_await_elidable]]`` 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_await_elidable]]``. - In caller coroutine, the return value of the callee is a prvalue that is immediately `co_await`ed. From the C++ perspective, it makes sense because we can ensure the lifetime of elided callee cannot exceed that of the caller if we can guarantee that the caller coroutine is never destroyed earlier than the callee coroutine. This is not generally true for any C++ programs. However, the library that implements `Task` types and executors may provide this guarantee to the compiler, providing the user with certainty that HALO will work on their programs. After this patch, when compiling coroutines that return a type with such attribute, the frontend checks that the type of the operand of `co_await` expressions (not `operator co_await`). If it's also attributed with `[[clang::coro_await_elidable]]`, the FE emits metadata on the call or invoke instruction as a hint for a later middle end pass to elide the elision. The original patch version is #94693 and as suggested, the patch is split into frontend and middle end solutions into stacked PRs. The middle end CoroSplit patch can be found at #99283 The middle end transformation that performs the elide can be found at #99285
yuxuanchen1997
added a commit
that referenced
this pull request
Sep 9, 2024
…ramp functions during CoroSplit (#99283) This patch is episode two of the coroutine HALO improvement project published on discourse: https://discourse.llvm.org/t/language-extension-for-better-more-deterministic-halo-for-c-coroutines/80044 Previously CoroElide depends on inlining, and its analysis does not work very well with code generated by the C++ frontend due the existence of many customization points. There has been issue reported to upstream how ineffective the original CoroElide was in real world applications. For C++ users, this set of patches aim to fix this problem by providing library authors and users deterministic HALO behaviour for some well-behaved coroutine `Task` types. The stack begins with a library side attribute on the `Task` class that guarantees no unstructured concurrency when coroutines are awaited directly with `co_await`ed as a prvalue. This attribute on Task types gives us lifetime guarantees and makes C++ FE capable to telling the ME which coroutine calls are elidable. We convey such information from FE through the attribute `coro_elide_safe`. This patch modifies CoroSplit to create a variant of the coroutine ramp function that 1) does not use heap allocated frame, instead take an additional parameter as the pointer to the frame. Such parameter is attributed with `dereferenceble` and `align` to convey size and align requirements for the frame. 2) always stores cleanup instead of destroy address for `coro.destroy()` actions. In a later patch, we will have a new pass that runs right after CoroSplit to find usages of the callee coroutine attributed `coro_elide_safe` in presplit coroutine callers, allocates the frame on its "stack", transform those usages to call the `noalloc` ramp function variant. (note I put quotes on the word "stack" here, because for presplit coroutine, any alloca will be spilled into the frame when it's being split) The C++ Frontend attribute implementation that works with this change can be found at #99282 The pass that makes use of the new `noalloc` split can be found at #99285
Base automatically changed from
users/yuxuanchen1997/coro-split-noalloc
to
main
September 9, 2024 06:09
yuxuanchen1997
deleted the
users/yuxuanchen1997/coro-transform-attr-must-elide
branch
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/16/builds/4972 Here is the relevant piece of the build log for the reference |
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/187/builds/1028 Here is the relevant piece of the build log for the reference |
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/14/builds/1362 Here is the relevant piece of the build log for the reference |
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/146/builds/1107 Here is the relevant piece of the build log for the reference |
|
Fix-forward for this patch: #107897 |
yuxuanchen1997
added a commit
that referenced
this pull request
Sep 10, 2024
…107897) After landing #99285 we found that the call graph update was causing the following crash when expensive checks are turned on ``` llvm-project/llvm/lib/Analysis/CGSCCPassManager.cpp:982: LazyCallGraph::SCC &updateCGAndAnalysisManagerForPass(LazyCallGraph &, LazyCallGraph::SCC &, LazyCallGraph::Node &, CGSCCAnalysisManager &, CGSCCUpdateResult &, FunctionAnalysisManager &, bool): Assertion `(RC == &TargetRC || RC->isAncestorOf(Targe tRC)) && "New call edge is not trivial!"' failed. ``` I have to admit I believe that the call graph update process I did for that patch could be wrong. After reading the code in `CGSCCToFunctionPassAdaptor`, I am convinced that `CoroAnnotationElidePass` can be a FunctionPass and rely on the adaptor to update the call graph for us, so long as we properly invalidate the caller's analyses. After this patch, `llvm/test/Transforms/Coroutines/coro-transform-must-elide.ll` no longer fails under expensive checks.
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/27/builds/398 Here is the relevant piece of the build log for the reference |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This patch is episode three of the middle end implementation for the coroutine HALO improvement project published on discourse: https://discourse.llvm.org/t/language-extension-for-better-more-deterministic-halo-for-c-coroutines/80044
After we attribute the calls to some coroutines as "coro_elide_safe" in the C++ FE and creating a
noallocramp function, we use a new middle end pass to move the call to coroutines to the noalloc variant.This pass should be run after CoroSplit. For each node we process in CoroSplit, we look for its callers and replace the attributed ones in presplit coroutines to the noalloc one. The transformed
noallocramp function will also require a frame pointer to a block of memory it can use as an activation frame. We allocate this on the caller's frame with an alloca.Please note that we cannot safely transform such attributed calls in post-split coroutines due to memory lifetime reasons. The CoroSplit pass is responsible for creating the coroutine frame spills for all the allocas in the coroutine. Therefore it will be unsafe to create new allocas like this one in post-split coroutines. This happens relatively rarely because CGSCC performs the passes on the callees before the caller. However, if multiple coroutines coexist in one SCC, this situation does happen (and prevents us from having potentially unbound frame size due to recursion.)
You can find episode 1: Clang FE of this patch series at #99282
Episode 2: CoroSplit at #99283