Author: ahatanak Date: Tue May 15 14:00:30 2018 New Revision: 332397 URL: http://llvm.org/viewvc/llvm-project?rev=332397&view=rev Log: Address post-commit review comments after r328731. NFC.
- Define a function (canPassInRegisters) that determines whether a record can be passed in registers based on language rules and target-specific ABI rules. - Set flag RecordDecl::ParamDestroyedInCallee to true in MSVC mode and remove ASTContext::isParamDestroyedInCallee, which is no longer needed. - Use the same type (unsigned) for RecordDecl's bit-field members. For more background, see the following discussions that took place on cfe-commits. http://lists.llvm.org/pipermail/cfe-commits/Week-of-Mon-20180326/223498.html http://lists.llvm.org/pipermail/cfe-commits/Week-of-Mon-20180402/223688.html http://lists.llvm.org/pipermail/cfe-commits/Week-of-Mon-20180409/224754.html http://lists.llvm.org/pipermail/cfe-commits/Week-of-Mon-20180423/226494.html http://lists.llvm.org/pipermail/cfe-commits/Week-of-Mon-20180507/227647.html Modified: cfe/trunk/include/clang/AST/ASTContext.h cfe/trunk/include/clang/AST/Decl.h cfe/trunk/lib/AST/ASTContext.cpp cfe/trunk/lib/CodeGen/CGCall.cpp cfe/trunk/lib/CodeGen/CGDecl.cpp cfe/trunk/lib/Sema/SemaChecking.cpp cfe/trunk/lib/Sema/SemaDeclCXX.cpp Modified: cfe/trunk/include/clang/AST/ASTContext.h URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/ASTContext.h?rev=332397&r1=332396&r2=332397&view=diff ============================================================================== --- cfe/trunk/include/clang/AST/ASTContext.h (original) +++ cfe/trunk/include/clang/AST/ASTContext.h Tue May 15 14:00:30 2018 @@ -1181,10 +1181,6 @@ public: const FunctionProtoType::ExceptionSpecInfo &ESI, bool AsWritten = false); - /// Determine whether a type is a class that should be detructed in the - /// callee function. - bool isParamDestroyedInCallee(QualType T) const; - /// Return the uniqued reference to the type for a complex /// number with the specified element type. QualType getComplexType(QualType T) const; Modified: cfe/trunk/include/clang/AST/Decl.h URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/Decl.h?rev=332397&r1=332396&r2=332397&view=diff ============================================================================== --- cfe/trunk/include/clang/AST/Decl.h (original) +++ cfe/trunk/include/clang/AST/Decl.h Tue May 15 14:00:30 2018 @@ -3572,40 +3572,38 @@ private: /// This is true if this struct ends with a flexible /// array member (e.g. int X[]) or if this union contains a struct that does. /// If so, this cannot be contained in arrays or other structs as a member. - bool HasFlexibleArrayMember : 1; + unsigned HasFlexibleArrayMember : 1; /// Whether this is the type of an anonymous struct or union. - bool AnonymousStructOrUnion : 1; + unsigned AnonymousStructOrUnion : 1; /// This is true if this struct has at least one member /// containing an Objective-C object pointer type. - bool HasObjectMember : 1; + unsigned HasObjectMember : 1; /// This is true if struct has at least one member of /// 'volatile' type. - bool HasVolatileMember : 1; + unsigned HasVolatileMember : 1; /// Whether the field declarations of this record have been loaded /// from external storage. To avoid unnecessary deserialization of /// methods/nested types we allow deserialization of just the fields /// when needed. - mutable bool LoadedFieldsFromExternalStorage : 1; + mutable unsigned LoadedFieldsFromExternalStorage : 1; /// Basic properties of non-trivial C structs. - bool NonTrivialToPrimitiveDefaultInitialize : 1; - bool NonTrivialToPrimitiveCopy : 1; - bool NonTrivialToPrimitiveDestroy : 1; - - /// Indicates whether this struct is destroyed in the callee. This flag is - /// meaningless when Microsoft ABI is used since parameters are always - /// destroyed in the callee. + unsigned NonTrivialToPrimitiveDefaultInitialize : 1; + unsigned NonTrivialToPrimitiveCopy : 1; + unsigned NonTrivialToPrimitiveDestroy : 1; + + /// Indicates whether this struct is destroyed in the callee. /// /// Please note that MSVC won't merge adjacent bitfields if they don't have /// the same type. - uint8_t ParamDestroyedInCallee : 1; + unsigned ParamDestroyedInCallee : 1; /// Represents the way this type is passed to a function. - uint8_t ArgPassingRestrictions : 2; + unsigned ArgPassingRestrictions : 2; protected: RecordDecl(Kind DK, TagKind TK, const ASTContext &C, DeclContext *DC, Modified: cfe/trunk/lib/AST/ASTContext.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/ASTContext.cpp?rev=332397&r1=332396&r2=332397&view=diff ============================================================================== --- cfe/trunk/lib/AST/ASTContext.cpp (original) +++ cfe/trunk/lib/AST/ASTContext.cpp Tue May 15 14:00:30 2018 @@ -2649,14 +2649,6 @@ void ASTContext::adjustExceptionSpec( } } -bool ASTContext::isParamDestroyedInCallee(QualType T) const { - if (getTargetInfo().getCXXABI().areArgsDestroyedLeftToRightInCallee()) - return true; - if (const auto *RT = T->getBaseElementTypeUnsafe()->getAs<RecordType>()) - return RT->getDecl()->isParamDestroyedInCallee(); - return false; -} - /// getComplexType - Return the uniqued reference to the type for a complex /// number with the specified element type. QualType ASTContext::getComplexType(QualType T) const { Modified: cfe/trunk/lib/CodeGen/CGCall.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGCall.cpp?rev=332397&r1=332396&r2=332397&view=diff ============================================================================== --- cfe/trunk/lib/CodeGen/CGCall.cpp (original) +++ cfe/trunk/lib/CodeGen/CGCall.cpp Tue May 15 14:00:30 2018 @@ -3071,7 +3071,8 @@ void CodeGenFunction::EmitDelegateCallAr // Deactivate the cleanup for the callee-destructed param that was pushed. if (hasAggregateEvaluationKind(type) && !CurFuncIsThunk && - getContext().isParamDestroyedInCallee(type) && type.isDestructedType()) { + type->getAs<RecordType>()->getDecl()->isParamDestroyedInCallee() && + type.isDestructedType()) { EHScopeStack::stable_iterator cleanup = CalleeDestructedParamCleanups.lookup(cast<ParmVarDecl>(param)); assert(cleanup.isValid() && @@ -3553,7 +3554,8 @@ void CodeGenFunction::EmitCallArg(CallAr // In the Microsoft C++ ABI, aggregate arguments are destructed by the callee. // However, we still have to push an EH-only cleanup in case we unwind before // we make it to the call. - if (HasAggregateEvalKind && getContext().isParamDestroyedInCallee(type)) { + if (HasAggregateEvalKind && + type->getAs<RecordType>()->getDecl()->isParamDestroyedInCallee()) { // If we're using inalloca, use the argument memory. Otherwise, use a // temporary. AggValueSlot Slot; Modified: cfe/trunk/lib/CodeGen/CGDecl.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGDecl.cpp?rev=332397&r1=332396&r2=332397&view=diff ============================================================================== --- cfe/trunk/lib/CodeGen/CGDecl.cpp (original) +++ cfe/trunk/lib/CodeGen/CGDecl.cpp Tue May 15 14:00:30 2018 @@ -1954,8 +1954,8 @@ void CodeGenFunction::EmitParmDecl(const // Push a destructor cleanup for this parameter if the ABI requires it. // Don't push a cleanup in a thunk for a method that will also emit a // cleanup. - if (!IsScalar && !CurFuncIsThunk && - getContext().isParamDestroyedInCallee(Ty)) { + if (hasAggregateEvaluationKind(Ty) && !CurFuncIsThunk && + Ty->getAs<RecordType>()->getDecl()->isParamDestroyedInCallee()) { if (QualType::DestructionKind DtorKind = Ty.isDestructedType()) { assert((DtorKind == QualType::DK_cxx_destructor || DtorKind == QualType::DK_nontrivial_c_struct) && Modified: cfe/trunk/lib/Sema/SemaChecking.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaChecking.cpp?rev=332397&r1=332396&r2=332397&view=diff ============================================================================== --- cfe/trunk/lib/Sema/SemaChecking.cpp (original) +++ cfe/trunk/lib/Sema/SemaChecking.cpp Tue May 15 14:00:30 2018 @@ -11288,7 +11288,7 @@ bool Sema::CheckParmsForFunctionDef(Arra if (!ClassDecl->isInvalidDecl() && !ClassDecl->hasIrrelevantDestructor() && !ClassDecl->isDependentContext() && - Context.isParamDestroyedInCallee(Param->getType())) { + ClassDecl->isParamDestroyedInCallee()) { CXXDestructorDecl *Destructor = LookupDestructor(ClassDecl); MarkFunctionReferenced(Param->getLocation(), Destructor); DiagnoseUseOfDecl(Destructor, Param->getLocation()); Modified: cfe/trunk/lib/Sema/SemaDeclCXX.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaDeclCXX.cpp?rev=332397&r1=332396&r2=332397&view=diff ============================================================================== --- cfe/trunk/lib/Sema/SemaDeclCXX.cpp (original) +++ cfe/trunk/lib/Sema/SemaDeclCXX.cpp Tue May 15 14:00:30 2018 @@ -5806,12 +5806,10 @@ static void DefineImplicitSpecialMember( } } -/// Determine whether a type would be destructed in the callee if it had a -/// non-trivial destructor. The rules here are based on C++ [class.temporary]p3, -/// which determines whether a struct can be passed to or returned from -/// functions in registers. -static bool paramCanBeDestroyedInCallee(Sema &S, CXXRecordDecl *D, - TargetInfo::CallingConvKind CCK) { +/// Determine whether a type is permitted to be passed or returned in +/// registers, per C++ [class.temporary]p3. +static bool canPassInRegisters(Sema &S, CXXRecordDecl *D, + TargetInfo::CallingConvKind CCK) { if (D->isDependentType() || D->isInvalidDecl()) return false; @@ -5821,6 +5819,63 @@ static bool paramCanBeDestroyedInCallee( return !D->hasNonTrivialDestructorForCall() && !D->hasNonTrivialCopyConstructorForCall(); + if (CCK == TargetInfo::CCK_MicrosoftX86_64) { + bool CopyCtorIsTrivial = false, CopyCtorIsTrivialForCall = false; + bool DtorIsTrivialForCall = false; + + // If a class has at least one non-deleted, trivial copy constructor, it + // is passed according to the C ABI. Otherwise, it is passed indirectly. + // + // Note: This permits classes with non-trivial copy or move ctors to be + // passed in registers, so long as they *also* have a trivial copy ctor, + // which is non-conforming. + if (D->needsImplicitCopyConstructor()) { + if (!D->defaultedCopyConstructorIsDeleted()) { + if (D->hasTrivialCopyConstructor()) + CopyCtorIsTrivial = true; + if (D->hasTrivialCopyConstructorForCall()) + CopyCtorIsTrivialForCall = true; + } + } else { + for (const CXXConstructorDecl *CD : D->ctors()) { + if (CD->isCopyConstructor() && !CD->isDeleted()) { + if (CD->isTrivial()) + CopyCtorIsTrivial = true; + if (CD->isTrivialForCall()) + CopyCtorIsTrivialForCall = true; + } + } + } + + if (D->needsImplicitDestructor()) { + if (!D->defaultedDestructorIsDeleted() && + D->hasTrivialDestructorForCall()) + DtorIsTrivialForCall = true; + } else if (const auto *DD = D->getDestructor()) { + if (!DD->isDeleted() && DD->isTrivialForCall()) + DtorIsTrivialForCall = true; + } + + // If the copy ctor and dtor are both trivial-for-calls, pass direct. + if (CopyCtorIsTrivialForCall && DtorIsTrivialForCall) + return true; + + // If a class has a destructor, we'd really like to pass it indirectly + // because it allows us to elide copies. Unfortunately, MSVC makes that + // impossible for small types, which it will pass in a single register or + // stack slot. Most objects with dtors are large-ish, so handle that early. + // We can't call out all large objects as being indirect because there are + // multiple x64 calling conventions and the C++ ABI code shouldn't dictate + // how we pass large POD types. + + // Note: This permits small classes with nontrivial destructors to be + // passed in registers, which is non-conforming. + if (CopyCtorIsTrivial && + S.getASTContext().getTypeSize(D->getTypeForDecl()) <= 64) + return true; + return false; + } + // Per C++ [class.temporary]p3, the relevant condition is: // each copy constructor, move constructor, and destructor of X is // either trivial or deleted, and X has at least one non-deleted copy @@ -5862,77 +5917,6 @@ static bool paramCanBeDestroyedInCallee( return HasNonDeletedCopyOrMove; } -static RecordDecl::ArgPassingKind -computeArgPassingRestrictions(bool DestroyedInCallee, const CXXRecordDecl *RD, - TargetInfo::CallingConvKind CCK, Sema &S) { - if (RD->isDependentType() || RD->isInvalidDecl()) - return RecordDecl::APK_CanPassInRegs; - - // The param cannot be passed in registers if ArgPassingRestrictions is set to - // APK_CanNeverPassInRegs. - if (RD->getArgPassingRestrictions() == RecordDecl::APK_CanNeverPassInRegs) - return RecordDecl::APK_CanNeverPassInRegs; - - if (CCK != TargetInfo::CCK_MicrosoftX86_64) - return DestroyedInCallee ? RecordDecl::APK_CanPassInRegs - : RecordDecl::APK_CannotPassInRegs; - - bool CopyCtorIsTrivial = false, CopyCtorIsTrivialForCall = false; - bool DtorIsTrivialForCall = false; - - // If a class has at least one non-deleted, trivial copy constructor, it - // is passed according to the C ABI. Otherwise, it is passed indirectly. - // - // Note: This permits classes with non-trivial copy or move ctors to be - // passed in registers, so long as they *also* have a trivial copy ctor, - // which is non-conforming. - if (RD->needsImplicitCopyConstructor()) { - if (!RD->defaultedCopyConstructorIsDeleted()) { - if (RD->hasTrivialCopyConstructor()) - CopyCtorIsTrivial = true; - if (RD->hasTrivialCopyConstructorForCall()) - CopyCtorIsTrivialForCall = true; - } - } else { - for (const CXXConstructorDecl *CD : RD->ctors()) { - if (CD->isCopyConstructor() && !CD->isDeleted()) { - if (CD->isTrivial()) - CopyCtorIsTrivial = true; - if (CD->isTrivialForCall()) - CopyCtorIsTrivialForCall = true; - } - } - } - - if (RD->needsImplicitDestructor()) { - if (!RD->defaultedDestructorIsDeleted() && - RD->hasTrivialDestructorForCall()) - DtorIsTrivialForCall = true; - } else if (const auto *D = RD->getDestructor()) { - if (!D->isDeleted() && D->isTrivialForCall()) - DtorIsTrivialForCall = true; - } - - // If the copy ctor and dtor are both trivial-for-calls, pass direct. - if (CopyCtorIsTrivialForCall && DtorIsTrivialForCall) - return RecordDecl::APK_CanPassInRegs; - - // If a class has a destructor, we'd really like to pass it indirectly - // because it allows us to elide copies. Unfortunately, MSVC makes that - // impossible for small types, which it will pass in a single register or - // stack slot. Most objects with dtors are large-ish, so handle that early. - // We can't call out all large objects as being indirect because there are - // multiple x64 calling conventions and the C++ ABI code shouldn't dictate - // how we pass large POD types. - - // Note: This permits small classes with nontrivial destructors to be - // passed in registers, which is non-conforming. - if (CopyCtorIsTrivial && - S.getASTContext().getTypeSize(RD->getTypeForDecl()) <= 64) - return RecordDecl::APK_CanPassInRegs; - return RecordDecl::APK_CannotPassInRegs; -} - /// Perform semantic checks on a class definition that has been /// completing, introducing implicitly-declared members, checking for /// abstract types, etc. @@ -6100,13 +6084,23 @@ void Sema::CheckCompletedCXXClass(CXXRec Context.getLangOpts().getClangABICompat() <= LangOptions::ClangABI::Ver4; TargetInfo::CallingConvKind CCK = Context.getTargetInfo().getCallingConvKind(ClangABICompat4); - bool DestroyedInCallee = paramCanBeDestroyedInCallee(*this, Record, CCK); - - if (Record->hasNonTrivialDestructor()) - Record->setParamDestroyedInCallee(DestroyedInCallee); + bool CanPass = canPassInRegisters(*this, Record, CCK); - Record->setArgPassingRestrictions( - computeArgPassingRestrictions(DestroyedInCallee, Record, CCK, *this)); + // Do not change ArgPassingRestrictions if it has already been set to + // APK_CanNeverPassInRegs. + if (Record->getArgPassingRestrictions() != RecordDecl::APK_CanNeverPassInRegs) + Record->setArgPassingRestrictions(CanPass + ? RecordDecl::APK_CanPassInRegs + : RecordDecl::APK_CannotPassInRegs); + + // If canPassInRegisters returns true despite the record having a non-trivial + // destructor, the record is destructed in the callee. This happens only when + // the record or one of its subobjects has a field annotated with trivial_abi + // or a field qualified with ObjC __strong/__weak. + if (Context.getTargetInfo().getCXXABI().areArgsDestroyedLeftToRightInCallee()) + Record->setParamDestroyedInCallee(true); + else if (Record->hasNonTrivialDestructor()) + Record->setParamDestroyedInCallee(CanPass); } /// Look up the special member function that would be called by a special _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
