Skip to content

Commit 9860622

Browse files
modocachemodocache
committedFeb 15, 2018
[Coroutines] Use allocator overload when available
Summary: Depends on https://reviews.llvm.org/D42605. An implementation of the behavior described in `[dcl.fct.def.coroutine]/7`: when a promise type overloads `operator new` using a "placement new" that takes the same argument types as the coroutine function, that overload is used when allocating the coroutine frame. Simply passing references to the coroutine function parameters directly to `operator new` results in invariant violations in LLVM's coroutine splitting pass, so this implementation modifies Clang codegen to produce allocator-specific alloc/store/loads for each parameter being forwarded to the allocator. Test Plan: `check-clang` Reviewers: rsmith, GorNishanov, eric_niebler Reviewed By: GorNishanov Subscribers: lewissbaker, EricWF, cfe-commits Differential Revision: https://reviews.llvm.org/D42606 llvm-svn: 325291
·
llvmorg-21-initllvmorg-7.0.0-rc1
1 parent f3f35ef commit 9860622

File tree

5 files changed

+220
-43
lines changed

5 files changed

+220
-43
lines changed
 

‎clang/lib/Sema/SemaCoroutine.cpp

Lines changed: 89 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@
1212
//===----------------------------------------------------------------------===//
1313

1414
#include "CoroutineStmtBuilder.h"
15+
#include "clang/AST/ASTLambda.h"
1516
#include "clang/AST/Decl.h"
1617
#include "clang/AST/ExprCXX.h"
1718
#include "clang/AST/StmtCXX.h"
@@ -506,24 +507,15 @@ VarDecl *Sema::buildCoroutinePromise(SourceLocation Loc) {
506507

507508
auto RefExpr = ExprEmpty();
508509
auto Move = Moves.find(PD);
509-
if (Move != Moves.end()) {
510-
// If a reference to the function parameter exists in the coroutine
511-
// frame, use that reference.
512-
auto *MoveDecl =
513-
cast<VarDecl>(cast<DeclStmt>(Move->second)->getSingleDecl());
514-
RefExpr = BuildDeclRefExpr(MoveDecl, MoveDecl->getType(),
515-
ExprValueKind::VK_LValue, FD->getLocation());
516-
} else {
517-
// If the function parameter doesn't exist in the coroutine frame, it
518-
// must be a scalar value. Use it directly.
519-
assert(!PD->getType()->getAsCXXRecordDecl() &&
520-
"Non-scalar types should have been moved and inserted into the "
521-
"parameter moves map");
522-
RefExpr =
523-
BuildDeclRefExpr(PD, PD->getOriginalType().getNonReferenceType(),
524-
ExprValueKind::VK_LValue, FD->getLocation());
525-
}
526-
510+
assert(Move != Moves.end() &&
511+
"Coroutine function parameter not inserted into move map");
512+
// If a reference to the function parameter exists in the coroutine
513+
// frame, use that reference.
514+
auto *MoveDecl =
515+
cast<VarDecl>(cast<DeclStmt>(Move->second)->getSingleDecl());
516+
RefExpr =
517+
BuildDeclRefExpr(MoveDecl, MoveDecl->getType().getNonReferenceType(),
518+
ExprValueKind::VK_LValue, FD->getLocation());
527519
if (RefExpr.isInvalid())
528520
return nullptr;
529521
CtorArgExprs.push_back(RefExpr.get());
@@ -1050,18 +1042,75 @@ bool CoroutineStmtBuilder::makeNewAndDeleteExpr() {
10501042

10511043
const bool RequiresNoThrowAlloc = ReturnStmtOnAllocFailure != nullptr;
10521044

1053-
// FIXME: Add support for stateful allocators.
1045+
// [dcl.fct.def.coroutine]/7
1046+
// Lookup allocation functions using a parameter list composed of the
1047+
// requested size of the coroutine state being allocated, followed by
1048+
// the coroutine function's arguments. If a matching allocation function
1049+
// exists, use it. Otherwise, use an allocation function that just takes
1050+
// the requested size.
10541051

10551052
FunctionDecl *OperatorNew = nullptr;
10561053
FunctionDecl *OperatorDelete = nullptr;
10571054
FunctionDecl *UnusedResult = nullptr;
10581055
bool PassAlignment = false;
10591056
SmallVector<Expr *, 1> PlacementArgs;
10601057

1058+
// [dcl.fct.def.coroutine]/7
1059+
// "The allocation function’s name is looked up in the scope of P.
1060+
// [...] If the lookup finds an allocation function in the scope of P,
1061+
// overload resolution is performed on a function call created by assembling
1062+
// an argument list. The first argument is the amount of space requested,
1063+
// and has type std::size_t. The lvalues p1 ... pn are the succeeding
1064+
// arguments."
1065+
//
1066+
// ...where "p1 ... pn" are defined earlier as:
1067+
//
1068+
// [dcl.fct.def.coroutine]/3
1069+
// "For a coroutine f that is a non-static member function, let P1 denote the
1070+
// type of the implicit object parameter (13.3.1) and P2 ... Pn be the types
1071+
// of the function parameters; otherwise let P1 ... Pn be the types of the
1072+
// function parameters. Let p1 ... pn be lvalues denoting those objects."
1073+
if (auto *MD = dyn_cast<CXXMethodDecl>(&FD)) {
1074+
if (MD->isInstance() && !isLambdaCallOperator(MD)) {
1075+
ExprResult ThisExpr = S.ActOnCXXThis(Loc);
1076+
if (ThisExpr.isInvalid())
1077+
return false;
1078+
ThisExpr = S.CreateBuiltinUnaryOp(Loc, UO_Deref, ThisExpr.get());
1079+
if (ThisExpr.isInvalid())
1080+
return false;
1081+
PlacementArgs.push_back(ThisExpr.get());
1082+
}
1083+
}
1084+
for (auto *PD : FD.parameters()) {
1085+
if (PD->getType()->isDependentType())
1086+
continue;
1087+
1088+
// Build a reference to the parameter.
1089+
auto PDLoc = PD->getLocation();
1090+
ExprResult PDRefExpr =
1091+
S.BuildDeclRefExpr(PD, PD->getOriginalType().getNonReferenceType(),
1092+
ExprValueKind::VK_LValue, PDLoc);
1093+
if (PDRefExpr.isInvalid())
1094+
return false;
1095+
1096+
PlacementArgs.push_back(PDRefExpr.get());
1097+
}
10611098
S.FindAllocationFunctions(Loc, SourceRange(),
10621099
/*UseGlobal*/ false, PromiseType,
10631100
/*isArray*/ false, PassAlignment, PlacementArgs,
1064-
OperatorNew, UnusedResult);
1101+
OperatorNew, UnusedResult, /*Diagnose*/ false);
1102+
1103+
// [dcl.fct.def.coroutine]/7
1104+
// "If no matching function is found, overload resolution is performed again
1105+
// on a function call created by passing just the amount of space required as
1106+
// an argument of type std::size_t."
1107+
if (!OperatorNew && !PlacementArgs.empty()) {
1108+
PlacementArgs.clear();
1109+
S.FindAllocationFunctions(Loc, SourceRange(),
1110+
/*UseGlobal*/ false, PromiseType,
1111+
/*isArray*/ false, PassAlignment,
1112+
PlacementArgs, OperatorNew, UnusedResult);
1113+
}
10651114

10661115
bool IsGlobalOverload =
10671116
OperatorNew && !isa<CXXRecordDecl>(OperatorNew->getDeclContext());
@@ -1080,7 +1129,8 @@ bool CoroutineStmtBuilder::makeNewAndDeleteExpr() {
10801129
OperatorNew, UnusedResult);
10811130
}
10821131

1083-
assert(OperatorNew && "expected definition of operator new to be found");
1132+
if (!OperatorNew)
1133+
return false;
10841134

10851135
if (RequiresNoThrowAlloc) {
10861136
const auto *FT = OperatorNew->getType()->getAs<FunctionProtoType>();
@@ -1386,25 +1436,28 @@ bool Sema::buildCoroutineParameterMoves(SourceLocation Loc) {
13861436
if (PD->getType()->isDependentType())
13871437
continue;
13881438

1389-
// No need to copy scalars, LLVM will take care of them.
1390-
if (PD->getType()->getAsCXXRecordDecl()) {
1391-
ExprResult PDRefExpr = BuildDeclRefExpr(
1392-
PD, PD->getType(), ExprValueKind::VK_LValue, Loc); // FIXME: scope?
1393-
if (PDRefExpr.isInvalid())
1394-
return false;
1439+
ExprResult PDRefExpr =
1440+
BuildDeclRefExpr(PD, PD->getType().getNonReferenceType(),
1441+
ExprValueKind::VK_LValue, Loc); // FIXME: scope?
1442+
if (PDRefExpr.isInvalid())
1443+
return false;
13951444

1396-
Expr *CExpr = castForMoving(*this, PDRefExpr.get());
1445+
Expr *CExpr = nullptr;
1446+
if (PD->getType()->getAsCXXRecordDecl() ||
1447+
PD->getType()->isRValueReferenceType())
1448+
CExpr = castForMoving(*this, PDRefExpr.get());
1449+
else
1450+
CExpr = PDRefExpr.get();
13971451

1398-
auto D = buildVarDecl(*this, Loc, PD->getType(), PD->getIdentifier());
1399-
AddInitializerToDecl(D, CExpr, /*DirectInit=*/true);
1452+
auto D = buildVarDecl(*this, Loc, PD->getType(), PD->getIdentifier());
1453+
AddInitializerToDecl(D, CExpr, /*DirectInit=*/true);
14001454

1401-
// Convert decl to a statement.
1402-
StmtResult Stmt = ActOnDeclStmt(ConvertDeclToDeclGroup(D), Loc, Loc);
1403-
if (Stmt.isInvalid())
1404-
return false;
1455+
// Convert decl to a statement.
1456+
StmtResult Stmt = ActOnDeclStmt(ConvertDeclToDeclGroup(D), Loc, Loc);
1457+
if (Stmt.isInvalid())
1458+
return false;
14051459

1406-
ScopeInfo->CoroutineParameterMoves.insert(std::make_pair(PD, Stmt.get()));
1407-
}
1460+
ScopeInfo->CoroutineParameterMoves.insert(std::make_pair(PD, Stmt.get()));
14081461
}
14091462
return true;
14101463
}

‎clang/test/CodeGenCoroutines/coro-alloc.cpp

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -106,6 +106,34 @@ extern "C" void f1(promise_new_tag ) {
106106
co_return;
107107
}
108108

109+
struct promise_matching_placement_new_tag {};
110+
111+
template<>
112+
struct std::experimental::coroutine_traits<void, promise_matching_placement_new_tag, int, float, double> {
113+
struct promise_type {
114+
void *operator new(unsigned long, promise_matching_placement_new_tag,
115+
int, float, double);
116+
void get_return_object() {}
117+
suspend_always initial_suspend() { return {}; }
118+
suspend_always final_suspend() { return {}; }
119+
void return_void() {}
120+
};
121+
};
122+
123+
// CHECK-LABEL: f1a(
124+
extern "C" void f1a(promise_matching_placement_new_tag, int x, float y , double z) {
125+
// CHECK: store i32 %x, i32* %x.addr, align 4
126+
// CHECK: store float %y, float* %y.addr, align 4
127+
// CHECK: store double %z, double* %z.addr, align 8
128+
// CHECK: %[[ID:.+]] = call token @llvm.coro.id(i32 16
129+
// CHECK: %[[SIZE:.+]] = call i64 @llvm.coro.size.i64()
130+
// CHECK: %[[INT:.+]] = load i32, i32* %x.addr, align 4
131+
// CHECK: %[[FLOAT:.+]] = load float, float* %y.addr, align 4
132+
// CHECK: %[[DOUBLE:.+]] = load double, double* %z.addr, align 8
133+
// CHECK: call i8* @_ZNSt12experimental16coroutine_traitsIJv34promise_matching_placement_new_tagifdEE12promise_typenwEmS1_ifd(i64 %[[SIZE]], i32 %[[INT]], float %[[FLOAT]], double %[[DOUBLE]])
134+
co_return;
135+
}
136+
109137
struct promise_delete_tag {};
110138

111139
template<>

‎clang/test/CodeGenCoroutines/coro-gro-nrvo.cpp

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -41,7 +41,7 @@ coro f(int) {
4141

4242
// CHECK: {{.*}}[[CoroInit]]:
4343
// CHECK: store i1 false, i1* %gro.active
44-
// CHECK-NEXT: call void @{{.*get_return_objectEv}}(%struct.coro* sret %agg.result
44+
// CHECK: call void @{{.*get_return_objectEv}}(%struct.coro* sret %agg.result
4545
// CHECK-NEXT: store i1 true, i1* %gro.active
4646
co_return;
4747
}
@@ -78,7 +78,7 @@ struct coro_two {
7878

7979
// CHECK: {{.*}}[[InitOnSuccess]]:
8080
// CHECK: store i1 false, i1* %gro.active
81-
// CHECK-NEXT: call void @{{.*get_return_objectEv}}(%struct.coro_two* sret %agg.result
81+
// CHECK: call void @{{.*get_return_objectEv}}(%struct.coro_two* sret %agg.result
8282
// CHECK-NEXT: store i1 true, i1* %gro.active
8383

8484
// CHECK: [[RetLabel]]:

‎clang/test/CodeGenCoroutines/coro-params.cpp

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -69,12 +69,12 @@ void f(int val, MoveOnly moParam, MoveAndCopy mcParam) {
6969
// CHECK: store i32 %val, i32* %[[ValAddr:.+]]
7070

7171
// CHECK: call i8* @llvm.coro.begin(
72-
// CHECK-NEXT: call void @_ZN8MoveOnlyC1EOS_(%struct.MoveOnly* %[[MoCopy]], %struct.MoveOnly* dereferenceable(4) %[[MoParam]])
72+
// CHECK: call void @_ZN8MoveOnlyC1EOS_(%struct.MoveOnly* %[[MoCopy]], %struct.MoveOnly* dereferenceable(4) %[[MoParam]])
7373
// CHECK-NEXT: call void @_ZN11MoveAndCopyC1EOS_(%struct.MoveAndCopy* %[[McCopy]], %struct.MoveAndCopy* dereferenceable(4) %[[McParam]]) #
7474
// CHECK-NEXT: invoke void @_ZNSt12experimental16coroutine_traitsIJvi8MoveOnly11MoveAndCopyEE12promise_typeC1Ev(
7575

7676
// CHECK: call void @_ZN14suspend_always12await_resumeEv(
77-
// CHECK: %[[IntParam:.+]] = load i32, i32* %val.addr
77+
// CHECK: %[[IntParam:.+]] = load i32, i32* %val1
7878
// CHECK: %[[MoGep:.+]] = getelementptr inbounds %struct.MoveOnly, %struct.MoveOnly* %[[MoCopy]], i32 0, i32 0
7979
// CHECK: %[[MoVal:.+]] = load i32, i32* %[[MoGep]]
8080
// CHECK: %[[McGep:.+]] = getelementptr inbounds %struct.MoveAndCopy, %struct.MoveAndCopy* %[[McCopy]], i32 0, i32 0
@@ -150,9 +150,9 @@ struct std::experimental::coroutine_traits<void, promise_matching_constructor, i
150150

151151
// CHECK-LABEL: void @_Z38coroutine_matching_promise_constructor28promise_matching_constructorifd(i32, float, double)
152152
void coroutine_matching_promise_constructor(promise_matching_constructor, int, float, double) {
153-
// CHECK: %[[INT:.+]] = load i32, i32* %.addr, align 4
154-
// CHECK: %[[FLOAT:.+]] = load float, float* %.addr1, align 4
155-
// CHECK: %[[DOUBLE:.+]] = load double, double* %.addr2, align 8
153+
// CHECK: %[[INT:.+]] = load i32, i32* %5, align 4
154+
// CHECK: %[[FLOAT:.+]] = load float, float* %6, align 4
155+
// CHECK: %[[DOUBLE:.+]] = load double, double* %7, align 8
156156
// CHECK: invoke void @_ZNSt12experimental16coroutine_traitsIJv28promise_matching_constructorifdEE12promise_typeC1ES1_ifd(%"struct.std::experimental::coroutine_traits<void, promise_matching_constructor, int, float, double>::promise_type"* %__promise, i32 %[[INT]], float %[[FLOAT]], double %[[DOUBLE]])
157157
co_return;
158158
}

‎clang/test/SemaCXX/coroutines.cpp

Lines changed: 96 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -781,6 +781,102 @@ coro<T> dependent_uses_nothrow_new(T) {
781781
}
782782
template coro<good_promise_13> dependent_uses_nothrow_new(good_promise_13);
783783

784+
struct good_promise_custom_new_operator {
785+
coro<good_promise_custom_new_operator> get_return_object();
786+
suspend_always initial_suspend();
787+
suspend_always final_suspend();
788+
void return_void();
789+
void unhandled_exception();
790+
void *operator new(unsigned long, double, float, int);
791+
};
792+
793+
coro<good_promise_custom_new_operator>
794+
good_coroutine_calls_custom_new_operator(double, float, int) {
795+
co_return;
796+
}
797+
798+
struct coroutine_nonstatic_member_struct;
799+
800+
struct good_promise_nonstatic_member_custom_new_operator {
801+
coro<good_promise_nonstatic_member_custom_new_operator> get_return_object();
802+
suspend_always initial_suspend();
803+
suspend_always final_suspend();
804+
void return_void();
805+
void unhandled_exception();
806+
void *operator new(unsigned long, coroutine_nonstatic_member_struct &, double);
807+
};
808+
809+
struct bad_promise_nonstatic_member_mismatched_custom_new_operator {
810+
coro<bad_promise_nonstatic_member_mismatched_custom_new_operator> get_return_object();
811+
suspend_always initial_suspend();
812+
suspend_always final_suspend();
813+
void return_void();
814+
void unhandled_exception();
815+
// expected-note@+1 {{candidate function not viable: requires 2 arguments, but 1 was provided}}
816+
void *operator new(unsigned long, double);
817+
};
818+
819+
struct coroutine_nonstatic_member_struct {
820+
coro<good_promise_nonstatic_member_custom_new_operator>
821+
good_coroutine_calls_nonstatic_member_custom_new_operator(double) {
822+
co_return;
823+
}
824+
825+
coro<bad_promise_nonstatic_member_mismatched_custom_new_operator>
826+
bad_coroutine_calls_nonstatic_member_mistmatched_custom_new_operator(double) {
827+
// expected-error@-1 {{no matching function for call to 'operator new'}}
828+
co_return;
829+
}
830+
};
831+
832+
struct bad_promise_mismatched_custom_new_operator {
833+
coro<bad_promise_mismatched_custom_new_operator> get_return_object();
834+
suspend_always initial_suspend();
835+
suspend_always final_suspend();
836+
void return_void();
837+
void unhandled_exception();
838+
// expected-note@+1 {{candidate function not viable: requires 4 arguments, but 1 was provided}}
839+
void *operator new(unsigned long, double, float, int);
840+
};
841+
842+
coro<bad_promise_mismatched_custom_new_operator>
843+
bad_coroutine_calls_mismatched_custom_new_operator(double) {
844+
// expected-error@-1 {{no matching function for call to 'operator new'}}
845+
co_return;
846+
}
847+
848+
struct bad_promise_throwing_custom_new_operator {
849+
static coro<bad_promise_throwing_custom_new_operator> get_return_object_on_allocation_failure();
850+
coro<bad_promise_throwing_custom_new_operator> get_return_object();
851+
suspend_always initial_suspend();
852+
suspend_always final_suspend();
853+
void return_void();
854+
void unhandled_exception();
855+
// expected-error@+1 {{'operator new' is required to have a non-throwing noexcept specification when the promise type declares 'get_return_object_on_allocation_failure()'}}
856+
void *operator new(unsigned long, double, float, int);
857+
};
858+
859+
coro<bad_promise_throwing_custom_new_operator>
860+
bad_coroutine_calls_throwing_custom_new_operator(double, float, int) {
861+
// expected-note@-1 {{call to 'operator new' implicitly required by coroutine function here}}
862+
co_return;
863+
}
864+
865+
struct good_promise_noexcept_custom_new_operator {
866+
static coro<good_promise_noexcept_custom_new_operator> get_return_object_on_allocation_failure();
867+
coro<good_promise_noexcept_custom_new_operator> get_return_object();
868+
suspend_always initial_suspend();
869+
suspend_always final_suspend();
870+
void return_void();
871+
void unhandled_exception();
872+
void *operator new(unsigned long, double, float, int) noexcept;
873+
};
874+
875+
coro<good_promise_noexcept_custom_new_operator>
876+
good_coroutine_calls_noexcept_custom_new_operator(double, float, int) {
877+
co_return;
878+
}
879+
784880
struct mismatch_gro_type_tag1 {};
785881
template<>
786882
struct std::experimental::coroutine_traits<int, mismatch_gro_type_tag1> {

0 commit comments

Comments
 (0)
Please sign in to comment.