v.g.vassilev updated this revision to Diff 109932.
v.g.vassilev marked an inline comment as done.
v.g.vassilev added a comment.
We set the record's property denoting whether we can pass the decl by registers
as a last step of `Sema::CheckCompletedCXXClass`. We cannot do it any earlier
than that because we have not computed the triviality information.
This patch still eagerly defines too many implicit members. As discussed with
@rsmith, he will take care of the non-trivial refactoring of
`Sema::ShouldDeleteSpecialMember` to fix this regression.
This patch passes all tests in codegen but still fails a few which are
sensitive to the amount of implicit members being created:
Failing Tests (13):
Clang :: CodeCompletion/ordinary-name-cxx11.cpp
Clang :: Misc/ast-dump-color.cpp
Clang :: Misc/ast-dump-decl.cpp
Clang :: Misc/ast-dump-invalid.cpp
Clang-Unit ::
AST/./ASTTests/CXXMethodDecl.CXXMethodDeclWithNoExceptSpecification
Clang-Unit ::
ASTMatchers/./ASTMatchersTests/ConstructorDeclaration.IsImplicit
Clang-Unit :: ASTMatchers/./ASTMatchersTests/ConstructorDeclaration.Kinds
Clang-Unit :: ASTMatchers/./ASTMatchersTests/DeclarationMatcher.MatchNot
Clang-Unit :: ASTMatchers/./ASTMatchersTests/DeclarationMatcher.hasMethod
Clang-Unit ::
ASTMatchers/./ASTMatchersTests/DeclaratorDecl.MatchesDeclaratorDecls
Clang-Unit :: ASTMatchers/./ASTMatchersTests/Matcher.References
Clang-Unit :: ASTMatchers/./ASTMatchersTests/TypeMatcher.MatchesClassType
Clang-Unit ::
ASTMatchers/Dynamic/./DynamicASTMatchersTests/RegistryTest.VariadicOp
Expected Passes : 10786
Expected Failures : 17
Unsupported Tests : 195
Unexpected Failures: 13
https://reviews.llvm.org/D35056
Files:
include/clang/AST/DeclCXX.h
include/clang/Sema/Sema.h
lib/AST/ASTImporter.cpp
lib/AST/DeclCXX.cpp
lib/CodeGen/CGCXXABI.cpp
lib/CodeGen/ItaniumCXXABI.cpp
lib/Sema/SemaDeclCXX.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
@@ -52,12 +52,11 @@
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"*)
+// CHECK-LABEL: define void @_ZN9move_ctor3barEv()
+// CHECK: call void @_Z{{.*}}C1Ev(
+// CHECK-NOT: call
+// CHECK: call void @_ZN9move_ctor3fooENS_1AE(%"struct.move_ctor::A"* %{{.*}})
+// CHECK-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"*)
}
@@ -73,12 +72,11 @@
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"*)
+// CHECK-LABEL: define void @_ZN11all_deleted3barEv()
+// CHECK: call void @_Z{{.*}}C1Ev(
+// CHECK-NOT: call
+// CHECK: call void @_ZN11all_deleted3fooENS_1AE(%"struct.all_deleted::A"* %{{.*}})
+// CHECK-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"*)
}
@@ -93,12 +91,11 @@
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"*)
+// CHECK-LABEL: define void @_ZN18implicitly_deleted3barEv()
+// CHECK: call void @_Z{{.*}}C1Ev(
+// 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@implicitly_deleted@@YAXUA@1@@Z"(%"struct.implicitly_deleted::A"*)
}
@@ -113,12 +110,11 @@
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"*)
+// CHECK-LABEL: define void @_ZN11one_deleted3barEv()
+// CHECK: call void @_Z{{.*}}C1Ev(
+// CHECK-NOT: call
+// CHECK: call void @_ZN11one_deleted3fooENS_1AE(%"struct.one_deleted::A"* %{{.*}})
+// CHECK-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"*)
}
@@ -195,12 +191,10 @@
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"*)
+// CHECK-LABEL: define void @_ZN14two_copy_ctors3barEv()
+// CHECK: call void @_Z{{.*}}C1Ev(
+// CHECK: call void @_ZN14two_copy_ctors3fooENS_1BE(%"struct.two_copy_ctors::B"* %{{.*}})
+// CHECK-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"*)
}
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/SemaDeclCXX.cpp
===================================================================
--- lib/Sema/SemaDeclCXX.cpp
+++ lib/Sema/SemaDeclCXX.cpp
@@ -5726,6 +5726,69 @@
}
}
+// non-trivial for the purposes of calls
+//
+// A type is considered non-trivial for the purposes of calls if:
+// (1) it has a non-trivial copy constructor, move constructor, or destructor,
+// or
+// (2) all of its copy and move constructors are deleted.
+//
+// This definition, as applied to class types, is intended to be the complement
+// of the definition in [class.temporary]p3 of types for which an extra
+// temporary is allowed when passing or returning a type. A type which is
+// trivial for the purposes of the ABI will be passed and returned according to
+// the rules of the underlying C ABI, e.g. in registers; often this has the
+// effect of performing a trivial copy of the type.
+bool Sema::CanPassInRegisters(CXXRecordDecl *D) {
+ if (D->isDependentType())
+ return false;
+
+ // If D has a non-trivial copy constructor, move constructor, or destructor,
+ // we cannot pass in registers.
+ if (D->hasNonTrivialCopyConstructor() || D->hasNonTrivialMoveConstructor()
+ || D->hasNonTrivialDestructor())
+ return false;
+
+ // We can only pass in registers 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;
+ }
+ }
+
+ if (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 = DeclareImplicitCopyConstructor(D);
+ CopyDeleted
+ = ShouldDeleteSpecialMember(CopyCtor, Sema::CXXCopyConstructor);
+ // FIXME: Express the fact that we do no need implicit move ctor but we
+ // don't have one in a better way.
+ MoveDeleted |= !D->hasMoveConstructor();
+ }
+
+ if (D->needsImplicitMoveConstructor()) {
+ CXXConstructorDecl *MoveCtor = DeclareImplicitMoveConstructor(D);
+ MoveDeleted
+ = ShouldDeleteSpecialMember(MoveCtor, Sema::CXXMoveConstructor);
+ }
+ }
+
+ return !(MoveDeleted && CopyDeleted);
+}
+
/// \brief Perform semantic checks on a class definition that has been
/// completing, introducing implicitly-declared members, checking for
/// abstract types, etc.
@@ -5870,6 +5933,9 @@
}
checkClassLevelDLLAttribute(Record);
+
+ if (CanPassInRegisters(Record))
+ Record->setCanPassInRegisters();
}
/// Look up the special member function that would be called by a special
@@ -7496,8 +7562,7 @@
reinterpret_cast<Decl**>(FieldCollector->getCurFields()),
FieldCollector->getCurNumFields()), LBrac, RBrac, AttrList);
- CheckCompletedCXXClass(
- dyn_cast_or_null<CXXRecordDecl>(TagDecl));
+ CheckCompletedCXXClass(dyn_cast_or_null<CXXRecordDecl>(TagDecl));
}
/// AddImplicitlyDeclaredMembersToClass - Adds any implicitly-declared
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,9 @@
}
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),
@@ -1450,7 +1451,7 @@
void CXXRecordDecl::completeDefinition(CXXFinalOverriderMap *FinalOverriders) {
RecordDecl::completeDefinition();
-
+
// 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/Sema/Sema.h
===================================================================
--- include/clang/Sema/Sema.h
+++ include/clang/Sema/Sema.h
@@ -4751,6 +4751,8 @@
ArrayRef<SourceRange> DynamicExceptionRanges,
Expr *NoexceptExpr);
+ bool CanPassInRegisters(CXXRecordDecl *D);
+
class InheritedConstructorInfo;
/// \brief Determine if a special member function should have a deleted
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,18 @@
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 Set that we can pass this RecordDecl in registers.
+ void setCanPassInRegisters() {
+ assert(!data().CanPassInRegisters && "Already set!");
+ data().CanPassInRegisters = true;
+ }
+
/// \brief Determine whether this class has a non-literal or/ volatile type
/// non-static data member or base class.
bool hasNonLiteralTypeFieldsOrBases() const {
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
