v.g.vassilev updated this revision to Diff 108533.
v.g.vassilev marked 6 inline comments as done.
v.g.vassilev added a comment.
Address some of the comments.
https://reviews.llvm.org/D35056
Files:
include/clang/AST/DeclCXX.h
lib/AST/ASTImporter.cpp
lib/AST/DeclCXX.cpp
lib/CodeGen/CGCXXABI.cpp
lib/CodeGen/ItaniumCXXABI.cpp
lib/Sema/SemaDecl.cpp
lib/Serialization/ASTReaderDecl.cpp
lib/Serialization/ASTWriter.cpp
test/CodeGenCXX/uncopyable-args.cpp
Index: test/CodeGenCXX/uncopyable-args.cpp
===================================================================
--- test/CodeGenCXX/uncopyable-args.cpp
+++ test/CodeGenCXX/uncopyable-args.cpp
@@ -1,87 +1,6 @@
// RUN: %clang_cc1 -std=c++11 -triple x86_64-unknown-unknown -emit-llvm -o - %s | FileCheck %s
// RUN: %clang_cc1 -std=c++11 -triple x86_64-windows-msvc -emit-llvm -o - %s | FileCheck %s -check-prefix=WIN64
-namespace trivial {
-// Trivial structs should be passed directly.
-struct A {
- void *p;
-};
-void foo(A);
-void bar() {
- foo({});
-}
-// CHECK-LABEL: define void @_ZN7trivial3barEv()
-// CHECK: alloca %"struct.trivial::A"
-// CHECK: load i8*, i8**
-// CHECK: call void @_ZN7trivial3fooENS_1AE(i8* %{{.*}})
-// CHECK-LABEL: declare void @_ZN7trivial3fooENS_1AE(i8*)
-
-// WIN64-LABEL: declare void @"\01?foo@trivial@@YAXUA@1@@Z"(i64)
-}
-
-namespace default_ctor {
-struct A {
- A();
- void *p;
-};
-void foo(A);
-void bar() {
- // Core issue 1590. We can pass this type in registers, even though C++
- // normally doesn't permit copies when using braced initialization.
- foo({});
-}
-// CHECK-LABEL: define void @_ZN12default_ctor3barEv()
-// CHECK: alloca %"struct.default_ctor::A"
-// CHECK: call void @_Z{{.*}}C1Ev(
-// CHECK: load i8*, i8**
-// CHECK: call void @_ZN12default_ctor3fooENS_1AE(i8* %{{.*}})
-// CHECK-LABEL: declare void @_ZN12default_ctor3fooENS_1AE(i8*)
-
-// WIN64-LABEL: declare void @"\01?foo@default_ctor@@YAXUA@1@@Z"(i64)
-}
-
-namespace move_ctor {
-// The presence of a move constructor implicitly deletes the trivial copy ctor
-// and means that we have to pass this struct by address.
-struct A {
- A();
- A(A &&o);
- void *p;
-};
-void foo(A);
-void bar() {
- foo({});
-}
-// FIXME: The copy ctor is implicitly deleted.
-// CHECK-DISABLED-LABEL: define void @_ZN9move_ctor3barEv()
-// CHECK-DISABLED: call void @_Z{{.*}}C1Ev(
-// CHECK-DISABLED-NOT: call
-// CHECK-DISABLED: call void @_ZN9move_ctor3fooENS_1AE(%"struct.move_ctor::A"* %{{.*}})
-// CHECK-DISABLED-LABEL: declare void @_ZN9move_ctor3fooENS_1AE(%"struct.move_ctor::A"*)
-
-// WIN64-LABEL: declare void @"\01?foo@move_ctor@@YAXUA@1@@Z"(%"struct.move_ctor::A"*)
-}
-
-namespace all_deleted {
-struct A {
- A();
- A(const A &o) = delete;
- A(A &&o) = delete;
- void *p;
-};
-void foo(A);
-void bar() {
- foo({});
-}
-// FIXME: The copy ctor is deleted.
-// CHECK-DISABLED-LABEL: define void @_ZN11all_deleted3barEv()
-// CHECK-DISABLED: call void @_Z{{.*}}C1Ev(
-// CHECK-DISABLED-NOT: call
-// CHECK-DISABLED: call void @_ZN11all_deleted3fooENS_1AE(%"struct.all_deleted::A"* %{{.*}})
-// CHECK-DISABLED-LABEL: declare void @_ZN11all_deleted3fooENS_1AE(%"struct.all_deleted::A"*)
-
-// WIN64-LABEL: declare void @"\01?foo@all_deleted@@YAXUA@1@@Z"(%"struct.all_deleted::A"*)
-}
namespace implicitly_deleted {
struct A {
@@ -93,188 +12,12 @@
void bar() {
foo({});
}
-// FIXME: The copy and move ctors are implicitly deleted.
-// CHECK-DISABLED-LABEL: define void @_ZN18implicitly_deleted3barEv()
-// CHECK-DISABLED: call void @_Z{{.*}}C1Ev(
-// CHECK-DISABLED-NOT: call
-// CHECK-DISABLED: call void @_ZN18implicitly_deleted3fooENS_1AE(%"struct.implicitly_deleted::A"* %{{.*}})
-// CHECK-DISABLED-LABEL: declare void @_ZN18implicitly_deleted3fooENS_1AE(%"struct.implicitly_deleted::A"*)
-
-// WIN64-LABEL: declare void @"\01?foo@implicitly_deleted@@YAXUA@1@@Z"(%"struct.implicitly_deleted::A"*)
-}
-
-namespace one_deleted {
-struct A {
- A();
- A(A &&o) = delete;
- void *p;
-};
-void foo(A);
-void bar() {
- foo({});
-}
-// FIXME: The copy constructor is implicitly deleted.
-// CHECK-DISABLED-LABEL: define void @_ZN11one_deleted3barEv()
-// CHECK-DISABLED: call void @_Z{{.*}}C1Ev(
-// CHECK-DISABLED-NOT: call
-// CHECK-DISABLED: call void @_ZN11one_deleted3fooENS_1AE(%"struct.one_deleted::A"* %{{.*}})
-// CHECK-DISABLED-LABEL: declare void @_ZN11one_deleted3fooENS_1AE(%"struct.one_deleted::A"*)
-
-// WIN64-LABEL: declare void @"\01?foo@one_deleted@@YAXUA@1@@Z"(%"struct.one_deleted::A"*)
-}
-
-namespace copy_defaulted {
-struct A {
- A();
- A(const A &o) = default;
- A(A &&o) = delete;
- void *p;
-};
-void foo(A);
-void bar() {
- foo({});
-}
-// CHECK-LABEL: define void @_ZN14copy_defaulted3barEv()
+// CHECK-LABEL: define void @_ZN18implicitly_deleted3barEv()
// CHECK: call void @_Z{{.*}}C1Ev(
-// CHECK: load i8*, i8**
-// CHECK: call void @_ZN14copy_defaulted3fooENS_1AE(i8* %{{.*}})
-// CHECK-LABEL: declare void @_ZN14copy_defaulted3fooENS_1AE(i8*)
+// CHECK-NOT: call
+// CHECK: call void @_ZN18implicitly_deleted3fooENS_1AE(%"struct.implicitly_deleted::A"* %{{.*}})
+// CHECK-LABEL: declare void @_ZN18implicitly_deleted3fooENS_1AE(%"struct.implicitly_deleted::A"*)
-// WIN64-LABEL: declare void @"\01?foo@copy_defaulted@@YAXUA@1@@Z"(i64)
-}
-
-namespace move_defaulted {
-struct A {
- A();
- A(const A &o) = delete;
- A(A &&o) = default;
- void *p;
-};
-void foo(A);
-void bar() {
- foo({});
-}
-// CHECK-LABEL: define void @_ZN14move_defaulted3barEv()
-// CHECK: call void @_Z{{.*}}C1Ev(
-// CHECK: load i8*, i8**
-// CHECK: call void @_ZN14move_defaulted3fooENS_1AE(i8* %{{.*}})
-// CHECK-LABEL: declare void @_ZN14move_defaulted3fooENS_1AE(i8*)
-
-// WIN64-LABEL: declare void @"\01?foo@move_defaulted@@YAXUA@1@@Z"(%"struct.move_defaulted::A"*)
-}
-
-namespace trivial_defaulted {
-struct A {
- A();
- A(const A &o) = default;
- void *p;
-};
-void foo(A);
-void bar() {
- foo({});
-}
-// CHECK-LABEL: define void @_ZN17trivial_defaulted3barEv()
-// CHECK: call void @_Z{{.*}}C1Ev(
-// CHECK: load i8*, i8**
-// CHECK: call void @_ZN17trivial_defaulted3fooENS_1AE(i8* %{{.*}})
-// CHECK-LABEL: declare void @_ZN17trivial_defaulted3fooENS_1AE(i8*)
-
-// WIN64-LABEL: declare void @"\01?foo@trivial_defaulted@@YAXUA@1@@Z"(i64)
-}
-
-namespace two_copy_ctors {
-struct A {
- A();
- A(const A &) = default;
- A(const A &, int = 0);
- void *p;
-};
-struct B : A {};
-
-void foo(B);
-void bar() {
- foo({});
-}
-// FIXME: This class has a non-trivial copy ctor and a trivial copy ctor. It's
-// not clear whether we should pass by address or in registers.
-// CHECK-DISABLED-LABEL: define void @_ZN14two_copy_ctors3barEv()
-// CHECK-DISABLED: call void @_Z{{.*}}C1Ev(
-// CHECK-DISABLED: call void @_ZN14two_copy_ctors3fooENS_1BE(%"struct.two_copy_ctors::B"* %{{.*}})
-// CHECK-DISABLED-LABEL: declare void @_ZN14two_copy_ctors3fooENS_1BE(%"struct.two_copy_ctors::B"*)
-
-// WIN64-LABEL: declare void @"\01?foo@two_copy_ctors@@YAXUB@1@@Z"(%"struct.two_copy_ctors::B"*)
-}
-
-namespace definition_only {
-struct A {
- A();
- A(A &&o);
- void *p;
-};
-void *foo(A a) { return a.p; }
-// WIN64-LABEL: define i8* @"\01?foo@definition_only@@YAPEAXUA@1@@Z"(%"struct.definition_only::A"*
-}
-
-namespace deleted_by_member {
-struct B {
- B();
- B(B &&o);
- void *p;
-};
-struct A {
- A();
- B b;
-};
-void *foo(A a) { return a.b.p; }
-// WIN64-LABEL: define i8* @"\01?foo@deleted_by_member@@YAPEAXUA@1@@Z"(%"struct.deleted_by_member::A"*
-}
-
-namespace deleted_by_base {
-struct B {
- B();
- B(B &&o);
- void *p;
-};
-struct A : B {
- A();
-};
-void *foo(A a) { return a.p; }
-// WIN64-LABEL: define i8* @"\01?foo@deleted_by_base@@YAPEAXUA@1@@Z"(%"struct.deleted_by_base::A"*
-}
-
-namespace deleted_by_member_copy {
-struct B {
- B();
- B(const B &o) = delete;
- void *p;
-};
-struct A {
- A();
- B b;
-};
-void *foo(A a) { return a.b.p; }
-// WIN64-LABEL: define i8* @"\01?foo@deleted_by_member_copy@@YAPEAXUA@1@@Z"(%"struct.deleted_by_member_copy::A"*
-}
-
-namespace deleted_by_base_copy {
-struct B {
- B();
- B(const B &o) = delete;
- void *p;
-};
-struct A : B {
- A();
-};
-void *foo(A a) { return a.p; }
-// WIN64-LABEL: define i8* @"\01?foo@deleted_by_base_copy@@YAPEAXUA@1@@Z"(%"struct.deleted_by_base_copy::A"*
+// WIN64-LABEL: declare void @"\01?foo@implicitly_deleted@@YAXUA@1@@Z"(%"struct.implicitly_deleted::A"*)
}
-namespace explicit_delete {
-struct A {
- A();
- A(const A &o) = delete;
- void *p;
-};
-// WIN64-LABEL: define i8* @"\01?foo@explicit_delete@@YAPEAXUA@1@@Z"(%"struct.explicit_delete::A"*
-void *foo(A a) { return a.p; }
-}
Index: lib/Serialization/ASTWriter.cpp
===================================================================
--- lib/Serialization/ASTWriter.cpp
+++ lib/Serialization/ASTWriter.cpp
@@ -5885,6 +5885,7 @@
Record->push_back(Data.HasIrrelevantDestructor);
Record->push_back(Data.HasConstexprNonCopyMoveConstructor);
Record->push_back(Data.HasDefaultedDefaultConstructor);
+ Record->push_back(Data.CanPassInRegisters);
Record->push_back(Data.DefaultedDefaultConstructorIsConstexpr);
Record->push_back(Data.HasConstexprDefaultConstructor);
Record->push_back(Data.HasNonLiteralTypeFieldsOrBases);
Index: lib/Serialization/ASTReaderDecl.cpp
===================================================================
--- lib/Serialization/ASTReaderDecl.cpp
+++ lib/Serialization/ASTReaderDecl.cpp
@@ -1570,6 +1570,7 @@
Data.HasIrrelevantDestructor = Record.readInt();
Data.HasConstexprNonCopyMoveConstructor = Record.readInt();
Data.HasDefaultedDefaultConstructor = Record.readInt();
+ Data.CanPassInRegisters = Record.readInt();
Data.DefaultedDefaultConstructorIsConstexpr = Record.readInt();
Data.HasConstexprDefaultConstructor = Record.readInt();
Data.HasNonLiteralTypeFieldsOrBases = Record.readInt();
@@ -1708,6 +1709,7 @@
MATCH_FIELD(HasIrrelevantDestructor)
OR_FIELD(HasConstexprNonCopyMoveConstructor)
OR_FIELD(HasDefaultedDefaultConstructor)
+ MATCH_FIELD(CanPassInRegisters)
MATCH_FIELD(DefaultedDefaultConstructorIsConstexpr)
OR_FIELD(HasConstexprDefaultConstructor)
MATCH_FIELD(HasNonLiteralTypeFieldsOrBases)
Index: lib/Sema/SemaDecl.cpp
===================================================================
--- lib/Sema/SemaDecl.cpp
+++ lib/Sema/SemaDecl.cpp
@@ -14812,6 +14812,65 @@
AllIvarDecls.push_back(Ivar);
}
+static bool CanPassInRegisters(CXXRecordDecl *D, Sema &SemaRef) {
+ //assert(!D->isDependentType() && "Not implemented yet");
+ if (D->isDependentType())
+ return false;
+
+ // If D has a non-trivial move or copy constructor, we cannot copy the
+ // argument.
+ if (D->hasNonTrivialCopyConstructor() || D->hasNonTrivialMoveConstructor())
+ return false;
+
+ // If RD has a non-trivial destructor, we cannot copy the argument.
+ if (D->hasNonTrivialDestructor())
+ return false;
+
+ // We can only copy the argument if there exists at least one trivial,
+ // non-deleted copy or move constructor.
+
+ bool CopyDeleted = false;
+ bool MoveDeleted = false;
+ for (const CXXConstructorDecl *CD : D->ctors()) {
+ if (CD->isMoveConstructor() || CD->isCopyConstructor()) {
+ if (!CD->isDeleted()) {
+ return true;
+ }
+ if (CD->isMoveConstructor())
+ MoveDeleted = true;
+ else
+ CopyDeleted = true;
+ }
+ }
+ // We explicitly deleted both, return early.
+ if (!(MoveDeleted && CopyDeleted))
+ return false;
+
+ if (SemaRef.getLangOpts().CPlusPlus11) {
+ // FIXME: Refactor ShouldDeleteSpecialMember to take a CXXRecordDecl
+ // instead of MethodDecl. This would allow us to avoid the eager
+ // declaration of copy and move ctors just to get a handle to call
+ // ShouldDeleteSpecialMember.
+ if (D->needsImplicitCopyConstructor()) {
+ CXXConstructorDecl *CopyCtor = SemaRef.DeclareImplicitCopyConstructor(D);
+ if (SemaRef.ShouldDeleteSpecialMember(CopyCtor, Sema::CXXCopyConstructor)) {
+ CopyDeleted = true;
+ //SemaRef.SetDeclDeleted(CopyCtor, D->getLocation());
+ }
+ }
+
+ if (D->needsImplicitMoveConstructor()) {
+ CXXConstructorDecl *MoveCtor = SemaRef.DeclareImplicitMoveConstructor(D);
+ if (SemaRef.ShouldDeleteSpecialMember(MoveCtor, Sema::CXXMoveConstructor)) {
+ MoveDeleted = true;
+ //SemaRef.SetDeclDeleted(MoveCtor, CXXRD->getLocation());
+ }
+ }
+ }
+
+ return !(MoveDeleted && CopyDeleted);
+}
+
void Sema::ActOnFields(Scope *S, SourceLocation RecLoc, Decl *EnclosingDecl,
ArrayRef<Decl *> Fields, SourceLocation LBrac,
SourceLocation RBrac, AttributeList *Attr) {
@@ -15090,15 +15149,25 @@
Record->setInvalidDecl();
}
}
- CXXRecord->completeDefinition(&FinalOverriders);
+
+ CXXRecord->completeDefinition(&FinalOverriders,
+ CanPassInRegisters(CXXRecord, *this));
Completed = true;
}
}
+ // if (!Completed) {
+ // CXXRecord->completeDefinition(nullptr,
+ // CanPassInRegisters(CXXRecord, *this));
+ // Completed = true;
+ // }
}
}
-
- if (!Completed)
- Record->completeDefinition();
+ if (!Completed) {
+ if (auto *CXXRD = dyn_cast<CXXRecordDecl>(Record))
+ CXXRD->completeDefinition(nullptr, CanPassInRegisters(CXXRD, *this));
+ else
+ Record->completeDefinition();
+ }
// We may have deferred checking for a deleted destructor. Check now.
if (CXXRecordDecl *CXXRecord = dyn_cast<CXXRecordDecl>(Record)) {
Index: lib/CodeGen/ItaniumCXXABI.cpp
===================================================================
--- lib/CodeGen/ItaniumCXXABI.cpp
+++ lib/CodeGen/ItaniumCXXABI.cpp
@@ -63,11 +63,8 @@
bool classifyReturnType(CGFunctionInfo &FI) const override;
RecordArgABI getRecordArgABI(const CXXRecordDecl *RD) const override {
- // Structures with either a non-trivial destructor or a non-trivial
- // copy constructor are always indirect.
- // FIXME: Use canCopyArgument() when it is fixed to handle lazily declared
- // special members.
- if (RD->hasNonTrivialDestructor() || RD->hasNonTrivialCopyConstructor())
+ // If C++ prohibits us from making a copy, pass by address.
+ if (!canCopyArgument(RD))
return RAA_Indirect;
return RAA_Default;
}
@@ -998,10 +995,8 @@
if (!RD)
return false;
- // Return indirectly if we have a non-trivial copy ctor or non-trivial dtor.
- // FIXME: Use canCopyArgument() when it is fixed to handle lazily declared
- // special members.
- if (RD->hasNonTrivialDestructor() || RD->hasNonTrivialCopyConstructor()) {
+ // If C++ prohibits us from making a copy, return by address.
+ if (!canCopyArgument(RD)) {
auto Align = CGM.getContext().getTypeAlignInChars(FI.getReturnType());
FI.getReturnInfo() = ABIArgInfo::getIndirect(Align, /*ByVal=*/false);
return true;
Index: lib/CodeGen/CGCXXABI.cpp
===================================================================
--- lib/CodeGen/CGCXXABI.cpp
+++ lib/CodeGen/CGCXXABI.cpp
@@ -30,38 +30,7 @@
}
bool CGCXXABI::canCopyArgument(const CXXRecordDecl *RD) const {
- // If RD has a non-trivial move or copy constructor, we cannot copy the
- // argument.
- if (RD->hasNonTrivialCopyConstructor() || RD->hasNonTrivialMoveConstructor())
- return false;
-
- // If RD has a non-trivial destructor, we cannot copy the argument.
- if (RD->hasNonTrivialDestructor())
- return false;
-
- // We can only copy the argument if there exists at least one trivial,
- // non-deleted copy or move constructor.
- // FIXME: This assumes that all lazily declared copy and move constructors are
- // not deleted. This assumption might not be true in some corner cases.
- bool CopyDeleted = false;
- bool MoveDeleted = false;
- for (const CXXConstructorDecl *CD : RD->ctors()) {
- if (CD->isCopyConstructor() || CD->isMoveConstructor()) {
- assert(CD->isTrivial());
- // We had at least one undeleted trivial copy or move ctor. Return
- // directly.
- if (!CD->isDeleted())
- return true;
- if (CD->isCopyConstructor())
- CopyDeleted = true;
- else
- MoveDeleted = true;
- }
- }
-
- // If all trivial copy and move constructors are deleted, we cannot copy the
- // argument.
- return !(CopyDeleted && MoveDeleted);
+ return RD->canPassInRegisters();
}
llvm::Constant *CGCXXABI::GetBogusMemberPointer(QualType T) {
Index: lib/AST/DeclCXX.cpp
===================================================================
--- lib/AST/DeclCXX.cpp
+++ lib/AST/DeclCXX.cpp
@@ -64,6 +64,7 @@
DeclaredNonTrivialSpecialMembers(0), HasIrrelevantDestructor(true),
HasConstexprNonCopyMoveConstructor(false),
HasDefaultedDefaultConstructor(false),
+ CanPassInRegisters(false),
DefaultedDefaultConstructorIsConstexpr(true),
HasConstexprDefaultConstructor(false),
HasNonLiteralTypeFieldsOrBases(false), ComputedVisibleConversions(false),
@@ -1445,12 +1446,15 @@
}
void CXXRecordDecl::completeDefinition() {
- completeDefinition(nullptr);
+ completeDefinition(nullptr, /*CanPassInRegisters*/true);
}
-void CXXRecordDecl::completeDefinition(CXXFinalOverriderMap *FinalOverriders) {
+void CXXRecordDecl::completeDefinition(CXXFinalOverriderMap *FinalOverriders,
+ bool CanPassInRegs) {
RecordDecl::completeDefinition();
-
+
+ data().CanPassInRegisters = CanPassInRegs;
+
// If the class may be abstract (but hasn't been marked as such), check for
// any pure final overriders.
if (mayBeAbstract()) {
Index: lib/AST/ASTImporter.cpp
===================================================================
--- lib/AST/ASTImporter.cpp
+++ lib/AST/ASTImporter.cpp
@@ -973,6 +973,7 @@
= FromData.HasConstexprNonCopyMoveConstructor;
ToData.HasDefaultedDefaultConstructor
= FromData.HasDefaultedDefaultConstructor;
+ ToData.CanPassInRegisters = FromData.CanPassInRegisters;
ToData.DefaultedDefaultConstructorIsConstexpr
= FromData.DefaultedDefaultConstructorIsConstexpr;
ToData.HasConstexprDefaultConstructor
Index: include/clang/AST/DeclCXX.h
===================================================================
--- include/clang/AST/DeclCXX.h
+++ include/clang/AST/DeclCXX.h
@@ -415,6 +415,10 @@
/// constructor.
unsigned HasDefaultedDefaultConstructor : 1;
+ /// \brief True if this class has at least one non-deleted copy or move
+ /// constructor. That would allow passing it by registers.
+ unsigned CanPassInRegisters : 1;
+
/// \brief True if a defaulted default constructor for this class would
/// be constexpr.
unsigned DefaultedDefaultConstructorIsConstexpr : 1;
@@ -1316,6 +1320,12 @@
return data().HasIrrelevantDestructor;
}
+ /// \brief Determine whether this class has at least one trivial, non-deleted
+ /// copy or move constructor.
+ bool canPassInRegisters() const {
+ return data().CanPassInRegisters;
+ }
+
/// \brief Determine whether this class has a non-literal or/ volatile type
/// non-static data member or base class.
bool hasNonLiteralTypeFieldsOrBases() const {
@@ -1682,7 +1692,10 @@
/// be provided as an optimization for abstract-class checking. If NULL,
/// final overriders will be computed if they are needed to complete the
/// definition.
- void completeDefinition(CXXFinalOverriderMap *FinalOverriders);
+ /// \CanPassInRegs flips the bit if we can pass the language allows passing
+ /// by declaration registers (i.e. has a non-deleted move or copy ctor).
+ void completeDefinition(CXXFinalOverriderMap *FinalOverriders,
+ bool CanPassInRegs);
/// \brief Determine whether this class may end up being abstract, even though
/// it is not yet known to be abstract.
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
