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. 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: 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/CodeGen/CGCXXABI.cpp =================================================================== --- lib/CodeGen/CGCXXABI.cpp +++ lib/CodeGen/CGCXXABI.cpp @@ -30,6 +30,9 @@ } bool CGCXXABI::canCopyArgument(const CXXRecordDecl *RD) const { + // See CanPassInRegisters in SemaDecl.cpp. These two functions + // should be kept consistent. + // If RD has a non-trivial move or copy constructor, we cannot copy the // argument. if (RD->hasNonTrivialCopyConstructor() || RD->hasNonTrivialMoveConstructor()) @@ -41,27 +44,7 @@ // 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/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/Sema/SemaDecl.cpp =================================================================== --- lib/Sema/SemaDecl.cpp +++ lib/Sema/SemaDecl.cpp @@ -14812,6 +14812,71 @@ AllIvarDecls.push_back(Ivar); } +// 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. +static bool CanPassInRegisters(CXXRecordDecl *D, Sema &SemaRef) { + if (D->isDependentType()) + return false; + + // (1) is handled in CGCXXABI::canCopyArgument because triviality of the ctors + // is computed in CheckCompletedCXXClass(D), which happens after call. + // FIXME: Figure out how to merge the content of CGCXXABI::canCopyArgument in + // here. This will require calling CheckCompletedCXXClass in ActOnFields, + // or here: + // SemaRef.CheckCompletedCXXClass(D); + + + // 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; + } + } + + 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); + CopyDeleted + = SemaRef.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 = SemaRef.DeclareImplicitMoveConstructor(D); + MoveDeleted + = SemaRef.ShouldDeleteSpecialMember(MoveCtor, Sema::CXXMoveConstructor); + } + } + + return !(MoveDeleted && CopyDeleted); +} + void Sema::ActOnFields(Scope *S, SourceLocation RecLoc, Decl *EnclosingDecl, ArrayRef Fields, SourceLocation LBrac, SourceLocation RBrac, AttributeList *Attr) { @@ -15090,14 +15155,20 @@ 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) + else if (!Completed) Record->completeDefinition(); // We may have deferred checking for a deleted destructor. Check now. 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/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: 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"*) }