Skip to content

Commit 7488b18

Browse files
yuxuanchen1997yuxuanchen1997
committed
[Clang] address upstream comments, use new name and Sema approach
1 parent 093cd09 commit 7488b18

File tree

10 files changed

+101
-30
lines changed

10 files changed

+101
-30
lines changed

‎clang/include/clang/AST/Expr.h

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3127,6 +3127,13 @@ class CallExpr : public Expr {
31273127
return getUnusedResultAttr(Ctx) != nullptr;
31283128
}
31293129

3130+
bool isCoroutineInplaceTaskCall() const {
3131+
return CallExprBits.IsCoroutineInplaceTaskCall;
3132+
}
3133+
void setIsCoroutineInplaceTaskCall(bool value = true) {
3134+
CallExprBits.IsCoroutineInplaceTaskCall = value;
3135+
}
3136+
31303137
SourceLocation getRParenLoc() const { return RParenLoc; }
31313138
void setRParenLoc(SourceLocation L) { RParenLoc = L; }
31323139

‎clang/include/clang/AST/ExprCXX.h

Lines changed: 14 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -5082,18 +5082,19 @@ class CoroutineSuspendExpr : public Expr {
50825082
enum SubExpr { Operand, Common, Ready, Suspend, Resume, Count };
50835083

50845084
Stmt *SubExprs[SubExpr::Count];
5085-
OpaqueValueExpr *OpaqueValue = nullptr;
5085+
OpaqueValueExpr *CommonExprOpaqueValue = nullptr;
5086+
OpaqueValueExpr *OperandOpaqueValue = nullptr;
50865087

50875088
public:
50885089
// These types correspond to the three C++ 'await_suspend' return variants
50895090
enum class SuspendReturnType { SuspendVoid, SuspendBool, SuspendHandle };
50905091

50915092
CoroutineSuspendExpr(StmtClass SC, SourceLocation KeywordLoc, Expr *Operand,
50925093
Expr *Common, Expr *Ready, Expr *Suspend, Expr *Resume,
5093-
OpaqueValueExpr *OpaqueValue)
5094+
OpaqueValueExpr *CommonExprOpaqueValue)
50945095
: Expr(SC, Resume->getType(), Resume->getValueKind(),
50955096
Resume->getObjectKind()),
5096-
KeywordLoc(KeywordLoc), OpaqueValue(OpaqueValue) {
5097+
KeywordLoc(KeywordLoc), CommonExprOpaqueValue(CommonExprOpaqueValue) {
50975098
SubExprs[SubExpr::Operand] = Operand;
50985099
SubExprs[SubExpr::Common] = Common;
50995100
SubExprs[SubExpr::Ready] = Ready;
@@ -5128,7 +5129,12 @@ class CoroutineSuspendExpr : public Expr {
51285129
}
51295130

51305131
/// getOpaqueValue - Return the opaque value placeholder.
5131-
OpaqueValueExpr *getOpaqueValue() const { return OpaqueValue; }
5132+
OpaqueValueExpr *getCommonExprOpaqueValue() const {
5133+
return CommonExprOpaqueValue;
5134+
}
5135+
5136+
OpaqueValueExpr *getOperandOpaqueValue() const { return OperandOpaqueValue; }
5137+
void setOperandOpaqueValue(OpaqueValueExpr *E) { OperandOpaqueValue = E; }
51325138

51335139
Expr *getReadyExpr() const {
51345140
return static_cast<Expr*>(SubExprs[SubExpr::Ready]);
@@ -5194,9 +5200,9 @@ class CoawaitExpr : public CoroutineSuspendExpr {
51945200
public:
51955201
CoawaitExpr(SourceLocation CoawaitLoc, Expr *Operand, Expr *Common,
51965202
Expr *Ready, Expr *Suspend, Expr *Resume,
5197-
OpaqueValueExpr *OpaqueValue, bool IsImplicit = false)
5203+
OpaqueValueExpr *CommonExprOpaqueValue, bool IsImplicit = false)
51985204
: CoroutineSuspendExpr(CoawaitExprClass, CoawaitLoc, Operand, Common,
5199-
Ready, Suspend, Resume, OpaqueValue) {
5205+
Ready, Suspend, Resume, CommonExprOpaqueValue) {
52005206
CoawaitBits.IsImplicit = IsImplicit;
52015207
}
52025208

@@ -5280,9 +5286,9 @@ class CoyieldExpr : public CoroutineSuspendExpr {
52805286
public:
52815287
CoyieldExpr(SourceLocation CoyieldLoc, Expr *Operand, Expr *Common,
52825288
Expr *Ready, Expr *Suspend, Expr *Resume,
5283-
OpaqueValueExpr *OpaqueValue)
5289+
OpaqueValueExpr *CommonExprOpaqueValue)
52845290
: CoroutineSuspendExpr(CoyieldExprClass, CoyieldLoc, Operand, Common,
5285-
Ready, Suspend, Resume, OpaqueValue) {}
5291+
Ready, Suspend, Resume, CommonExprOpaqueValue) {}
52865292
CoyieldExpr(SourceLocation CoyieldLoc, QualType Ty, Expr *Operand,
52875293
Expr *Common)
52885294
: CoroutineSuspendExpr(CoyieldExprClass, CoyieldLoc, Ty, Operand,

‎clang/include/clang/AST/Stmt.h

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -561,8 +561,11 @@ class alignas(void *) Stmt {
561561
LLVM_PREFERRED_TYPE(bool)
562562
unsigned HasFPFeatures : 1;
563563

564+
LLVM_PREFERRED_TYPE(bool)
565+
unsigned IsCoroutineInplaceTaskCall : 1;
566+
564567
/// Padding used to align OffsetToTrailingObjects to a byte multiple.
565-
unsigned : 24 - 3 - NumExprBits;
568+
unsigned : 24 - 4 - NumExprBits;
566569

567570
/// The offset in bytes from the this pointer to the start of the
568571
/// trailing objects belonging to CallExpr. Intentionally byte sized
@@ -1163,6 +1166,8 @@ class alignas(void *) Stmt {
11631166

11641167
LLVM_PREFERRED_TYPE(bool)
11651168
unsigned IsImplicit : 1;
1169+
1170+
LLVM_PREFERRED_TYPE(bool)
11661171
unsigned IsInplaceCall : 1;
11671172
};
11681173

‎clang/lib/CodeGen/CGCoroutine.cpp

Lines changed: 26 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@
1515
#include "llvm/ADT/ScopeExit.h"
1616
#include "clang/AST/StmtCXX.h"
1717
#include "clang/AST/StmtVisitor.h"
18+
#include "llvm/IR/Intrinsics.h"
1819

1920
using namespace clang;
2021
using namespace CodeGen;
@@ -224,9 +225,26 @@ static LValueOrRValue emitSuspendExpression(CodeGenFunction &CGF, CGCoroData &Co
224225
AwaitKind Kind, AggValueSlot aggSlot,
225226
bool ignoreResult, bool forLValue) {
226227
auto *E = S.getCommonExpr();
228+
auto &Builder = CGF.Builder;
227229

228-
auto CommonBinder =
229-
CodeGenFunction::OpaqueValueMappingData::bind(CGF, S.getOpaqueValue(), E);
230+
// S.getOperandOpaqueValue() may be null, in this case it maps to nothing.
231+
std::optional<CodeGenFunction::OpaqueValueMapping> OperandMapping = std::nullopt;
232+
auto CallOV = S.getOperandOpaqueValue();
233+
if (CallOV) {
234+
OperandMapping.emplace(CGF, CallOV);
235+
LValue LV = CGF.getOrCreateOpaqueLValueMapping(CallOV);
236+
llvm::Value *Value = LV.getPointer(CGF);
237+
// for (auto *U : Value->users()) {
238+
// if (auto *Call = cast<llvm::CallBase>(U)) {
239+
// Call->dump();
240+
// }
241+
// }
242+
auto SafeElide = CGF.CGM.getIntrinsic(llvm::Intrinsic::coro_safe_elide);
243+
if (cast<CallExpr>(CallOV->getSourceExpr())->isCoroutineInplaceTaskCall())
244+
Builder.CreateCall(SafeElide, Value);
245+
}
246+
auto CommonBinder = CodeGenFunction::OpaqueValueMappingData::bind(
247+
CGF, S.getCommonExprOpaqueValue(), E);
230248
auto UnbindCommonOnExit =
231249
llvm::make_scope_exit([&] { CommonBinder.unbind(CGF); });
232250

@@ -241,7 +259,6 @@ static LValueOrRValue emitSuspendExpression(CodeGenFunction &CGF, CGCoroData &Co
241259
// Otherwise, emit suspend logic.
242260
CGF.EmitBlock(SuspendBlock);
243261

244-
auto &Builder = CGF.Builder;
245262
llvm::Function *CoroSave = CGF.CGM.getIntrinsic(llvm::Intrinsic::coro_save);
246263
auto *NullPtr = llvm::ConstantPointerNull::get(CGF.CGM.Int8PtrTy);
247264
auto *SaveCall = Builder.CreateCall(CoroSave, {NullPtr});
@@ -256,7 +273,8 @@ static LValueOrRValue emitSuspendExpression(CodeGenFunction &CGF, CGCoroData &Co
256273

257274
SmallVector<llvm::Value *, 3> SuspendIntrinsicCallArgs;
258275
SuspendIntrinsicCallArgs.push_back(
259-
CGF.getOrCreateOpaqueLValueMapping(S.getOpaqueValue()).getPointer(CGF));
276+
CGF.getOrCreateOpaqueLValueMapping(S.getCommonExprOpaqueValue())
277+
.getPointer(CGF));
260278

261279
SuspendIntrinsicCallArgs.push_back(CGF.CurCoro.Data->CoroBegin);
262280
SuspendIntrinsicCallArgs.push_back(SuspendWrapper);
@@ -455,7 +473,7 @@ CodeGenFunction::generateAwaitSuspendWrapper(Twine const &CoroName,
455473
Builder.CreateLoad(GetAddrOfLocalVar(&FrameDecl));
456474

457475
auto AwaiterBinder = CodeGenFunction::OpaqueValueMappingData::bind(
458-
*this, S.getOpaqueValue(), AwaiterLValue);
476+
*this, S.getCommonExprOpaqueValue(), AwaiterLValue);
459477

460478
auto *SuspendRet = EmitScalarExpr(S.getSuspendExpr());
461479

@@ -473,6 +491,9 @@ CodeGenFunction::generateAwaitSuspendWrapper(Twine const &CoroName,
473491

474492
LValue
475493
CodeGenFunction::EmitCoawaitLValue(const CoawaitExpr *E) {
494+
if (E->isInplaceCall()) {
495+
llvm::dbgs() << "Inplace call!\n";
496+
}
476497
assert(getCoroutineSuspendExprReturnType(getContext(), E)->isReferenceType() &&
477498
"Can't have a scalar return unless the return type is a "
478499
"reference type!");

‎clang/lib/CodeGen/CGExpr.cpp

Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -618,11 +618,7 @@ EmitMaterializeTemporaryExpr(const MaterializeTemporaryExpr *M) {
618618
}
619619
}
620620

621-
auto Ret = MakeAddrLValue(Object, M->getType(), AlignmentSource::Decl);
622-
if (TemporaryValues.contains(M)) {
623-
TemporaryValues[M] = Ret.getPointer(*this);
624-
}
625-
return Ret;
621+
return MakeAddrLValue(Object, M->getType(), AlignmentSource::Decl);
626622
}
627623

628624
RValue

‎clang/lib/CodeGen/CodeGenFunction.h

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -369,9 +369,6 @@ class CodeGenFunction : public CodeGenTypeCache {
369369
};
370370
CGCoroInfo CurCoro;
371371

372-
llvm::SmallDenseMap<const MaterializeTemporaryExpr *, llvm::Value *>
373-
TemporaryValues;
374-
375372
bool isCoroutine() const {
376373
return CurCoro.Data != nullptr;
377374
}

‎clang/lib/Sema/SemaCoroutine.cpp

Lines changed: 38 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@
1515

1616
#include "CoroutineStmtBuilder.h"
1717
#include "clang/AST/ASTLambda.h"
18+
#include "clang/AST/ComputeDependence.h"
1819
#include "clang/AST/Decl.h"
1920
#include "clang/AST/Expr.h"
2021
#include "clang/AST/ExprCXX.h"
@@ -838,6 +839,19 @@ static bool isCoroInplaceCall(Expr *Operand) {
838839
return isAttributedCoroInplaceTask(Operand->getType());
839840
}
840841

842+
template <typename DesiredExpr>
843+
DesiredExpr *getExprWrappedByTemporary(Expr *E) {
844+
if (auto *BTE = dyn_cast<CXXBindTemporaryExpr>(E)) {
845+
E = BTE->getSubExpr();
846+
}
847+
848+
if (auto *S = dyn_cast<DesiredExpr>(E)) {
849+
return S;
850+
}
851+
852+
return nullptr;
853+
}
854+
841855
// Attempts to resolve and build a CoawaitExpr from "raw" inputs, bailing out to
842856
// DependentCoawaitExpr if needed.
843857
ExprResult Sema::BuildUnresolvedCoawaitExpr(SourceLocation Loc, Expr *Operand,
@@ -861,6 +875,26 @@ ExprResult Sema::BuildUnresolvedCoawaitExpr(SourceLocation Loc, Expr *Operand,
861875
}
862876

863877
auto *RD = Promise->getType()->getAsCXXRecordDecl();
878+
bool InplaceCall =
879+
isCoroInplaceCall(Operand) &&
880+
isAttributedCoroInplaceTask(
881+
getCurFunctionDecl(/*AllowLambda=*/true)->getReturnType());
882+
883+
OpaqueValueExpr *OpaqueCallExpr = nullptr;
884+
885+
if (InplaceCall) {
886+
if (auto *Temporary = dyn_cast<CXXBindTemporaryExpr>(Operand)) {
887+
auto *SubExpr = Temporary->getSubExpr();
888+
if (CallExpr *Call = dyn_cast<CallExpr>(SubExpr)) {
889+
Call->setIsCoroutineInplaceTaskCall();
890+
OpaqueCallExpr = new (Context)
891+
OpaqueValueExpr(Call->getRParenLoc(), Call->getType(),
892+
Call->getValueKind(), Call->getObjectKind(), Call);
893+
Temporary->setSubExpr(OpaqueCallExpr);
894+
}
895+
}
896+
}
897+
864898
auto *Transformed = Operand;
865899
if (lookupMember(*this, "await_transform", RD, Loc)) {
866900
ExprResult R =
@@ -878,11 +912,11 @@ ExprResult Sema::BuildUnresolvedCoawaitExpr(SourceLocation Loc, Expr *Operand,
878912
return ExprError();
879913

880914
auto Res = BuildResolvedCoawaitExpr(Loc, Operand, Awaiter.get());
881-
if (!Res.isInvalid() && isCoroInplaceCall(Operand) &&
882-
isAttributedCoroInplaceTask(
883-
getCurFunctionDecl(/*AllowLambda=*/true)->getReturnType())) {
915+
if (!Res.isInvalid() && InplaceCall) {
884916
// BuildResolvedCoawaitExpr must return a CoawaitExpr, if valid.
885-
Res.getAs<CoawaitExpr>()->setIsInplaceCall();
917+
CoawaitExpr *CE = Res.getAs<CoawaitExpr>();
918+
CE->setIsInplaceCall();
919+
CE->setOperandOpaqueValue(OpaqueCallExpr);
886920
}
887921
return Res;
888922
}

‎clang/lib/Serialization/ASTReaderStmt.cpp

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -483,7 +483,9 @@ void ASTStmtReader::VisitCoawaitExpr(CoawaitExpr *E) {
483483
E->KeywordLoc = readSourceLocation();
484484
for (auto &SubExpr: E->SubExprs)
485485
SubExpr = Record.readSubStmt();
486-
E->OpaqueValue = cast_or_null<OpaqueValueExpr>(Record.readSubStmt());
486+
E->CommonExprOpaqueValue =
487+
cast_or_null<OpaqueValueExpr>(Record.readSubStmt());
488+
E->OperandOpaqueValue = cast_or_null<OpaqueValueExpr>(Record.readSubStmt());
487489
E->setIsImplicit(Record.readInt() != 0);
488490
}
489491

@@ -492,7 +494,9 @@ void ASTStmtReader::VisitCoyieldExpr(CoyieldExpr *E) {
492494
E->KeywordLoc = readSourceLocation();
493495
for (auto &SubExpr: E->SubExprs)
494496
SubExpr = Record.readSubStmt();
495-
E->OpaqueValue = cast_or_null<OpaqueValueExpr>(Record.readSubStmt());
497+
E->CommonExprOpaqueValue =
498+
cast_or_null<OpaqueValueExpr>(Record.readSubStmt());
499+
E->OperandOpaqueValue = cast_or_null<OpaqueValueExpr>(Record.readSubStmt());
496500
}
497501

498502
void ASTStmtReader::VisitDependentCoawaitExpr(DependentCoawaitExpr *E) {

‎clang/lib/Serialization/ASTWriterStmt.cpp

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -443,7 +443,8 @@ void ASTStmtWriter::VisitCoroutineSuspendExpr(CoroutineSuspendExpr *E) {
443443
Record.AddSourceLocation(E->getKeywordLoc());
444444
for (Stmt *S : E->children())
445445
Record.AddStmt(S);
446-
Record.AddStmt(E->getOpaqueValue());
446+
Record.AddStmt(E->getCommonExprOpaqueValue());
447+
Record.AddStmt(E->getOperandOpaqueValue());
447448
}
448449

449450
void ASTStmtWriter::VisitCoawaitExpr(CoawaitExpr *E) {

‎clang/test/Misc/pragma-attribute-supported-attributes-list.test

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -59,10 +59,10 @@
5959
// CHECK-NEXT: ConsumableSetOnRead (SubjectMatchRule_record)
6060
// CHECK-NEXT: Convergent (SubjectMatchRule_function)
6161
// CHECK-NEXT: CoroDisableLifetimeBound (SubjectMatchRule_function)
62+
// CHECK-NEXT: CoroInplaceTask (SubjectMatchRule_record)
6263
// CHECK-NEXT: CoroLifetimeBound (SubjectMatchRule_record)
6364
// CHECK-NEXT: CoroOnlyDestroyWhenComplete (SubjectMatchRule_record)
6465
// CHECK-NEXT: CoroReturnType (SubjectMatchRule_record)
65-
// CHECK-NEXT: CoroInplaceTask (SubjectMatchRule_record)
6666
// CHECK-NEXT: CoroWrapper (SubjectMatchRule_function)
6767
// CHECK-NEXT: DLLExport (SubjectMatchRule_function, SubjectMatchRule_variable, SubjectMatchRule_record, SubjectMatchRule_objc_interface)
6868
// CHECK-NEXT: DLLImport (SubjectMatchRule_function, SubjectMatchRule_variable, SubjectMatchRule_record, SubjectMatchRule_objc_interface)

0 commit comments

Comments
 (0)