mizvekov updated this revision to Diff 373323.
mizvekov edited the summary of this revision.
mizvekov added a comment.
add a couple more FIXMEs and a couple of asserts in the constexpr path for
rvalue copy elision.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D109800/new/
https://reviews.llvm.org/D109800
Files:
clang/include/clang/Sema/Initialization.h
clang/lib/AST/ExprConstant.cpp
clang/lib/CodeGen/CGExprCXX.cpp
clang/lib/Sema/Sema.cpp
clang/lib/Sema/SemaCoroutine.cpp
clang/lib/Sema/SemaDeclCXX.cpp
clang/lib/Sema/SemaExpr.cpp
clang/lib/Sema/SemaExprCXX.cpp
clang/lib/Sema/SemaLambda.cpp
clang/lib/Sema/SemaObjCProperty.cpp
clang/lib/Sema/SemaStmt.cpp
clang/test/CodeGen/nrvo-tracking.cpp
clang/test/CodeGenCXX/copy-elision.cpp
Index: clang/test/CodeGenCXX/copy-elision.cpp
===================================================================
--- /dev/null
+++ clang/test/CodeGenCXX/copy-elision.cpp
@@ -0,0 +1,34 @@
+// RUN: %clang_cc1 -std=c++14 -triple x86_64-unknown-unknown-gnu -emit-llvm -O1 -fexperimental-new-pass-manager -o - %s | FileCheck %s
+
+template <class T> T test() {
+ return T();
+}
+
+struct A {
+ A();
+ A(A &);
+ A(int);
+ operator int();
+};
+
+// FIXME: There should be copy elision here.
+// CHECK-LABEL: define{{.*}} void @_Z4testI1AET_v
+// CHECK: call void @_ZN1AC1Ev
+// CHECK-NEXT: call i32 @_ZN1AcviEv
+// CHECK-NEXT: call void @_ZN1AC1Ei
+// CHECK-NEXT: call void @llvm.lifetime.end
+template A test<A>();
+
+struct BSub {};
+struct B : BSub {
+ B();
+ B(B &);
+ B(const BSub &);
+};
+
+// FIXME: There should be copy elision here.
+// CHECK-LABEL: define{{.*}} void @_Z4testI1BET_v
+// CHECK: call void @_ZN1BC1Ev
+// CHECK: call void @_ZN1BC1ERK4BSub
+// CHECK-NEXT: call void @llvm.lifetime.end
+template B test<B>();
Index: clang/test/CodeGen/nrvo-tracking.cpp
===================================================================
--- clang/test/CodeGen/nrvo-tracking.cpp
+++ clang/test/CodeGen/nrvo-tracking.cpp
@@ -282,3 +282,40 @@
}
} // namespace test_alignas
+
+namespace PR51862 {
+
+template <class T> T test() {
+ T a;
+ T b;
+ if (0)
+ return a;
+ return b;
+}
+
+struct A {
+ A();
+ A(A &);
+ A(int);
+ operator int();
+};
+
+// CHECK-LABEL: define{{.*}} void @_ZN7PR518624testINS_1AEEET_v
+// CHECK: call i32 @_ZN7PR518621AcviEv
+// CHECK-NEXT: call void @_ZN7PR518621AC1Ei
+// CHECK-NEXT: call void @llvm.lifetime.end
+template A test<A>();
+
+struct BSub {};
+struct B : BSub {
+ B();
+ B(B &);
+ B(const BSub &);
+};
+
+// CHECK-LABEL: define{{.*}} void @_ZN7PR518624testINS_1BEEET_v
+// CHECK: call void @_ZN7PR518621BC1ERKNS_4BSubE
+// CHECK-NEXT: call void @llvm.lifetime.end
+template B test<B>();
+
+} // namespace PR51862
Index: clang/lib/Sema/SemaStmt.cpp
===================================================================
--- clang/lib/Sema/SemaStmt.cpp
+++ clang/lib/Sema/SemaStmt.cpp
@@ -3653,8 +3653,8 @@
// In C++ the return statement is handled via a copy initialization.
// the C version of which boils down to CheckSingleAssignmentConstraints.
- InitializedEntity Entity = InitializedEntity::InitializeResult(
- ReturnLoc, FnRetType, NRVOCandidate != nullptr);
+ InitializedEntity Entity =
+ InitializedEntity::InitializeResult(ReturnLoc, FnRetType);
ExprResult Res = PerformMoveOrCopyInitialization(
Entity, NRInfo, RetValExp, SupressSimplerImplicitMoves);
if (Res.isInvalid()) {
@@ -4085,8 +4085,8 @@
// the C version of which boils down to CheckSingleAssignmentConstraints.
if (!HasDependentReturnType && !RetValExp->isTypeDependent()) {
// we have a non-void function with an expression, continue checking
- InitializedEntity Entity = InitializedEntity::InitializeResult(
- ReturnLoc, RetType, NRVOCandidate != nullptr);
+ InitializedEntity Entity =
+ InitializedEntity::InitializeResult(ReturnLoc, RetType);
ExprResult Res = PerformMoveOrCopyInitialization(
Entity, NRInfo, RetValExp, SupressSimplerImplicitMoves);
if (Res.isInvalid()) {
Index: clang/lib/Sema/SemaObjCProperty.cpp
===================================================================
--- clang/lib/Sema/SemaObjCProperty.cpp
+++ clang/lib/Sema/SemaObjCProperty.cpp
@@ -1467,8 +1467,7 @@
LoadSelfExpr, true, true);
ExprResult Res = PerformCopyInitialization(
InitializedEntity::InitializeResult(PropertyDiagLoc,
- getterMethod->getReturnType(),
- /*NRVO=*/false),
+ getterMethod->getReturnType()),
PropertyDiagLoc, IvarRefExpr);
if (!Res.isInvalid()) {
Expr *ResExpr = Res.getAs<Expr>();
Index: clang/lib/Sema/SemaLambda.cpp
===================================================================
--- clang/lib/Sema/SemaLambda.cpp
+++ clang/lib/Sema/SemaLambda.cpp
@@ -1978,8 +1978,7 @@
CallOperator->markUsed(Context);
ExprResult Init = PerformCopyInitialization(
- InitializedEntity::InitializeLambdaToBlock(ConvLocation, Src->getType(),
- /*NRVO=*/false),
+ InitializedEntity::InitializeLambdaToBlock(ConvLocation, Src->getType()),
CurrentLocation, Src);
if (!Init.isInvalid())
Init = ActOnFinishFullExpr(Init.get(), /*DiscardedValue*/ false);
Index: clang/lib/Sema/SemaExprCXX.cpp
===================================================================
--- clang/lib/Sema/SemaExprCXX.cpp
+++ clang/lib/Sema/SemaExprCXX.cpp
@@ -893,9 +893,8 @@
if (CheckCXXThrowOperand(OpLoc, ExceptionObjectTy, Ex))
return ExprError();
- InitializedEntity Entity = InitializedEntity::InitializeException(
- OpLoc, ExceptionObjectTy,
- /*NRVO=*/NRInfo.isCopyElidable());
+ InitializedEntity Entity =
+ InitializedEntity::InitializeException(OpLoc, ExceptionObjectTy);
ExprResult Res = PerformMoveOrCopyInitialization(Entity, NRInfo, Ex);
if (Res.isInvalid())
return ExprError();
Index: clang/lib/Sema/SemaExpr.cpp
===================================================================
--- clang/lib/Sema/SemaExpr.cpp
+++ clang/lib/Sema/SemaExpr.cpp
@@ -15711,7 +15711,7 @@
if (!Result.isInvalid()) {
Result = PerformCopyInitialization(
InitializedEntity::InitializeBlock(Var->getLocation(),
- Cap.getCaptureType(), false),
+ Cap.getCaptureType()),
Loc, Result.get());
}
Index: clang/lib/Sema/SemaDeclCXX.cpp
===================================================================
--- clang/lib/Sema/SemaDeclCXX.cpp
+++ clang/lib/Sema/SemaDeclCXX.cpp
@@ -15300,8 +15300,17 @@
// can be omitted by constructing the temporary object
// directly into the target of the omitted copy/move
if (ConstructKind == CXXConstructExpr::CK_Complete && Constructor &&
+ // FIXME: Converting constructors should also be accepted.
+ // But to fix this, the logic that digs down into a CXXConstructExpr
+ // to find the source object needs to handle it.
+ // Right now it assumes the source object is passed directly as the
+ // first argument.
Constructor->isCopyOrMoveConstructor() && hasOneRealArgument(ExprArgs)) {
Expr *SubExpr = ExprArgs[0];
+ // FIXME: Per above, this is also incorrect if we want to accept
+ // converting constructors, as isTemporaryObject will
+ // reject temporaries with different type from the
+ // CXXRecord itself.
Elidable = SubExpr->isTemporaryObject(
Context, cast<CXXRecordDecl>(FoundDecl->getDeclContext()));
}
Index: clang/lib/Sema/SemaCoroutine.cpp
===================================================================
--- clang/lib/Sema/SemaCoroutine.cpp
+++ clang/lib/Sema/SemaCoroutine.cpp
@@ -1533,7 +1533,7 @@
if (GroType->isVoidType()) {
// Trigger a nice error message.
InitializedEntity Entity =
- InitializedEntity::InitializeResult(Loc, FnRetType, false);
+ InitializedEntity::InitializeResult(Loc, FnRetType);
S.PerformCopyInitialization(Entity, SourceLocation(), ReturnValue);
noteMemberDeclaredHere(S, ReturnValue, Fn);
return false;
Index: clang/lib/Sema/Sema.cpp
===================================================================
--- clang/lib/Sema/Sema.cpp
+++ clang/lib/Sema/Sema.cpp
@@ -2020,7 +2020,7 @@
Expr *VarRef =
new (S.Context) DeclRefExpr(S.Context, VD, false, T, VK_LValue, Loc);
ExprResult Result;
- auto IE = InitializedEntity::InitializeBlock(Loc, T, false);
+ auto IE = InitializedEntity::InitializeBlock(Loc, T);
if (S.getLangOpts().CPlusPlus2b) {
auto *E = ImplicitCastExpr::Create(S.Context, T, CK_NoOp, VarRef, nullptr,
VK_XValue, FPOptionsOverride());
Index: clang/lib/CodeGen/CGExprCXX.cpp
===================================================================
--- clang/lib/CodeGen/CGExprCXX.cpp
+++ clang/lib/CodeGen/CGExprCXX.cpp
@@ -609,15 +609,18 @@
return;
// Elide the constructor if we're constructing from a temporary.
- // The temporary check is required because Sema sets this on NRVO
- // returns.
if (getLangOpts().ElideConstructors && E->isElidable()) {
- assert(getContext().hasSameUnqualifiedType(E->getType(),
- E->getArg(0)->getType()));
- if (E->getArg(0)->isTemporaryObject(getContext(), CD->getParent())) {
- EmitAggExpr(E->getArg(0), Dest);
- return;
- }
+ // FIXME: This only handles the simples case, where the source object
+ // is passed directly as the first argument to the constructor.
+ // This should also handle stepping though implicit casts and
+ // conversion sequences which involve two steps, with a
+ // conversion operator followed by a converting constructor.
+ const Expr *SrcObj = E->getArg(0);
+ assert(SrcObj->isTemporaryObject(getContext(), CD->getParent()));
+ assert(
+ getContext().hasSameUnqualifiedType(E->getType(), SrcObj->getType()));
+ EmitAggExpr(SrcObj, Dest);
+ return;
}
if (const ArrayType *arrayType
Index: clang/lib/AST/ExprConstant.cpp
===================================================================
--- clang/lib/AST/ExprConstant.cpp
+++ clang/lib/AST/ExprConstant.cpp
@@ -9933,10 +9933,19 @@
return false;
// Avoid materializing a temporary for an elidable copy/move constructor.
- if (E->isElidable() && !ZeroInit)
- if (const MaterializeTemporaryExpr *ME
- = dyn_cast<MaterializeTemporaryExpr>(E->getArg(0)))
+ if (E->isElidable() && !ZeroInit) {
+ // FIXME: This only handles the simples case, where the source object
+ // is passed directly as the first argument to the constructor.
+ // This should also handle stepping though implicit casts and
+ // and conversion sequences which involve two steps, with a
+ // conversion operator followed by a converting constructor.
+ const Expr *SrcObj = E->getArg(0);
+ assert(SrcObj->isTemporaryObject(Info.Ctx, FD->getParent()));
+ assert(Info.Ctx.hasSameUnqualifiedType(E->getType(), SrcObj->getType()));
+ if (const MaterializeTemporaryExpr *ME =
+ dyn_cast<MaterializeTemporaryExpr>(SrcObj))
return Visit(ME->getSubExpr());
+ }
if (ZeroInit && !ZeroInitialization(E, T))
return false;
Index: clang/include/clang/Sema/Initialization.h
===================================================================
--- clang/include/clang/Sema/Initialization.h
+++ clang/include/clang/Sema/Initialization.h
@@ -298,8 +298,8 @@
/// Create the initialization entity for the result of a function.
static InitializedEntity InitializeResult(SourceLocation ReturnLoc,
- QualType Type, bool NRVO) {
- return InitializedEntity(EK_Result, ReturnLoc, Type, NRVO);
+ QualType Type) {
+ return InitializedEntity(EK_Result, ReturnLoc, Type);
}
static InitializedEntity InitializeStmtExprResult(SourceLocation ReturnLoc,
@@ -308,20 +308,20 @@
}
static InitializedEntity InitializeBlock(SourceLocation BlockVarLoc,
- QualType Type, bool NRVO) {
- return InitializedEntity(EK_BlockElement, BlockVarLoc, Type, NRVO);
+ QualType Type) {
+ return InitializedEntity(EK_BlockElement, BlockVarLoc, Type);
}
static InitializedEntity InitializeLambdaToBlock(SourceLocation BlockVarLoc,
- QualType Type, bool NRVO) {
+ QualType Type) {
return InitializedEntity(EK_LambdaToBlockConversionBlockElement,
- BlockVarLoc, Type, NRVO);
+ BlockVarLoc, Type);
}
/// Create the initialization entity for an exception object.
static InitializedEntity InitializeException(SourceLocation ThrowLoc,
- QualType Type, bool NRVO) {
- return InitializedEntity(EK_Exception, ThrowLoc, Type, NRVO);
+ QualType Type) {
+ return InitializedEntity(EK_Exception, ThrowLoc, Type);
}
/// Create the initialization entity for an object allocated via new.
_______________________________________________
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
