From d236a34ddb3f9e77147ba1da45c6cbfca8949a45 Mon Sep 17 00:00:00 2001 From: Akira Hatanaka Date: Mon, 9 Apr 2018 21:47:58 +0000 Subject: [PATCH] Revert "[ObjC++] Never pass structs that transitively contain __weak fields in" This reverts commit r329617. It broke a windows bot. http://lab.llvm.org:8011/builders/llvm-clang-lld-x86_64-scei-ps4-windows10pro-fast/builds/16372/steps/test/logs/stdio llvm-svn: 329627 --- clang/include/clang/AST/Decl.h | 45 ++++++------------------- clang/include/clang/AST/Type.h | 2 ++ clang/lib/AST/Decl.cpp | 2 +- clang/lib/AST/DeclCXX.cpp | 9 +---- clang/lib/AST/Type.cpp | 11 ++++++ clang/lib/Sema/SemaDecl.cpp | 9 ++--- clang/lib/Sema/SemaDeclCXX.cpp | 30 ++++++++--------- clang/lib/Serialization/ASTReaderDecl.cpp | 5 ++- clang/lib/Serialization/ASTWriter.cpp | 2 +- clang/lib/Serialization/ASTWriterDecl.cpp | 5 ++- clang/test/CodeGenObjCXX/objc-struct-cxx-abi.mm | 29 ---------------- 11 files changed, 47 insertions(+), 102 deletions(-) diff --git a/clang/include/clang/AST/Decl.h b/clang/include/clang/AST/Decl.h index 515ab3b..f9f5cf8 100644 --- a/clang/include/clang/AST/Decl.h +++ b/clang/include/clang/AST/Decl.h @@ -3541,31 +3541,6 @@ public: /// union Y { int A, B; }; // Has body with members A and B (FieldDecls). /// This decl will be marked invalid if *any* members are invalid. class RecordDecl : public TagDecl { -public: - /// Enum that represents the different ways arguments are passed to and - /// returned from function calls. This takes into account the target-specific - /// and version-specific rules along with the rules determined by the - /// language. - enum ArgPassingKind { - /// The argument of this type can be passed directly in registers. - APK_CanPassInRegs, - - /// The argument of this type cannot be passed directly in registers. - /// Records containing this type as a subobject are not forced to be passed - /// indirectly. This value is used only in C++. This value is required by - /// C++ because, in uncommon situations, it is possible for a class to have - /// only trivial copy/move constructors even when one of its subobjects has - /// a non-trivial copy/move constructor (if e.g. the corresponding copy/move - /// constructor in the derived class is deleted). - APK_CannotPassInRegs, - - /// The argument of this type cannot be passed directly in registers. - /// Records containing this type as a subobject are forced to be passed - /// indirectly. - APK_CanNeverPassInRegs - }; - -private: friend class DeclContext; // FIXME: This can be packed into the bitfields in Decl. @@ -3596,14 +3571,17 @@ private: bool NonTrivialToPrimitiveCopy : 1; bool NonTrivialToPrimitiveDestroy : 1; + /// True if this class can be passed in a non-address-preserving fashion + /// (such as in registers). + /// This does not imply anything about how the ABI in use will actually + /// pass an object of this class. + bool CanPassInRegisters : 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. bool ParamDestroyedInCallee : 1; - /// Represents the way this type is passed to a function. - ArgPassingKind ArgPassingRestrictions : 2; - protected: RecordDecl(Kind DK, TagKind TK, const ASTContext &C, DeclContext *DC, SourceLocation StartLoc, SourceLocation IdLoc, @@ -3691,15 +3669,12 @@ public: /// it must have at least one trivial, non-deleted copy or move constructor. /// FIXME: This should be set as part of completeDefinition. bool canPassInRegisters() const { - return ArgPassingRestrictions == APK_CanPassInRegs; - } - - ArgPassingKind getArgPassingRestrictions() const { - return ArgPassingRestrictions; + return CanPassInRegisters; } - void setArgPassingRestrictions(ArgPassingKind Kind) { - ArgPassingRestrictions = Kind; + /// Set that we can pass this RecordDecl in registers. + void setCanPassInRegisters(bool CanPass) { + CanPassInRegisters = CanPass; } bool isParamDestroyedInCallee() const { diff --git a/clang/include/clang/AST/Type.h b/clang/include/clang/AST/Type.h index ab6e113..a97f5af 100644 --- a/clang/include/clang/AST/Type.h +++ b/clang/include/clang/AST/Type.h @@ -1149,6 +1149,8 @@ public: /// source object is placed in an uninitialized state. PrimitiveCopyKind isNonTrivialToPrimitiveDestructiveMove() const; + bool canPassInRegisters() const; + enum DestructionKind { DK_none, DK_cxx_destructor, diff --git a/clang/lib/AST/Decl.cpp b/clang/lib/AST/Decl.cpp index 18ef94b..b49fda0 100644 --- a/clang/lib/AST/Decl.cpp +++ b/clang/lib/AST/Decl.cpp @@ -3956,7 +3956,7 @@ RecordDecl::RecordDecl(Kind DK, TagKind TK, const ASTContext &C, LoadedFieldsFromExternalStorage(false), NonTrivialToPrimitiveDefaultInitialize(false), NonTrivialToPrimitiveCopy(false), NonTrivialToPrimitiveDestroy(false), - ParamDestroyedInCallee(false), ArgPassingRestrictions(APK_CanPassInRegs) { + CanPassInRegisters(true), ParamDestroyedInCallee(false) { assert(classof(static_cast(this)) && "Invalid Kind!"); } diff --git a/clang/lib/AST/DeclCXX.cpp b/clang/lib/AST/DeclCXX.cpp index dd653d8..99ff1ca 100644 --- a/clang/lib/AST/DeclCXX.cpp +++ b/clang/lib/AST/DeclCXX.cpp @@ -421,10 +421,6 @@ CXXRecordDecl::setBases(CXXBaseSpecifier const * const *Bases, if (BaseClassDecl->hasVolatileMember()) setHasVolatileMember(true); - if (BaseClassDecl->getArgPassingRestrictions() == - RecordDecl::APK_CanNeverPassInRegs) - setArgPassingRestrictions(RecordDecl::APK_CanNeverPassInRegs); - // Keep track of the presence of mutable fields. if (BaseClassDecl->hasMutableFields()) { data().HasMutableFields = true; @@ -954,7 +950,7 @@ void CXXRecordDecl::addedMember(Decl *D) { // Structs with __weak fields should never be passed directly. if (LT == Qualifiers::OCL_Weak) - setArgPassingRestrictions(RecordDecl::APK_CanNeverPassInRegs); + setCanPassInRegisters(false); Data.HasIrrelevantDestructor = false; } else if (!Context.getLangOpts().ObjCAutoRefCount) { @@ -1121,9 +1117,6 @@ void CXXRecordDecl::addedMember(Decl *D) { setHasObjectMember(true); if (FieldRec->hasVolatileMember()) setHasVolatileMember(true); - if (FieldRec->getArgPassingRestrictions() == - RecordDecl::APK_CanNeverPassInRegs) - setArgPassingRestrictions(RecordDecl::APK_CanNeverPassInRegs); // C++0x [class]p7: // A standard-layout class is a class that: diff --git a/clang/lib/AST/Type.cpp b/clang/lib/AST/Type.cpp index a2a6077..cca5ddc 100644 --- a/clang/lib/AST/Type.cpp +++ b/clang/lib/AST/Type.cpp @@ -2239,6 +2239,17 @@ QualType::isNonTrivialToPrimitiveDestructiveMove() const { return isNonTrivialToPrimitiveCopy(); } +bool QualType::canPassInRegisters() const { + if (const auto *RT = + getTypePtr()->getBaseElementTypeUnsafe()->getAs()) + return RT->getDecl()->canPassInRegisters(); + + if (getQualifiers().getObjCLifetime() == Qualifiers::OCL_Weak) + return false; + + return true; +} + bool Type::isLiteralType(const ASTContext &Ctx) const { if (isDependentType()) return false; diff --git a/clang/lib/Sema/SemaDecl.cpp b/clang/lib/Sema/SemaDecl.cpp index 9228d65..f650e04 100644 --- a/clang/lib/Sema/SemaDecl.cpp +++ b/clang/lib/Sema/SemaDecl.cpp @@ -15465,13 +15465,8 @@ void Sema::ActOnFields(Scope *S, SourceLocation RecLoc, Decl *EnclosingDecl, Record->setNonTrivialToPrimitiveDestroy(true); Record->setParamDestroyedInCallee(true); } - - if (const auto *RT = FT->getAs()) { - if (RT->getDecl()->getArgPassingRestrictions() == - RecordDecl::APK_CanNeverPassInRegs) - Record->setArgPassingRestrictions(RecordDecl::APK_CanNeverPassInRegs); - } else if (FT.getQualifiers().getObjCLifetime() == Qualifiers::OCL_Weak) - Record->setArgPassingRestrictions(RecordDecl::APK_CanNeverPassInRegs); + if (!FT.canPassInRegisters()) + Record->setCanPassInRegisters(false); } if (Record && FD->getType().isVolatileQualified()) diff --git a/clang/lib/Sema/SemaDeclCXX.cpp b/clang/lib/Sema/SemaDeclCXX.cpp index 08d3bc2..55c7a9a 100644 --- a/clang/lib/Sema/SemaDeclCXX.cpp +++ b/clang/lib/Sema/SemaDeclCXX.cpp @@ -5847,20 +5847,20 @@ static bool paramCanBeDestroyedInCallee(Sema &S, CXXRecordDecl *D, return HasNonDeletedCopyOrMove; } -static RecordDecl::ArgPassingKind -computeArgPassingRestrictions(bool DestroyedInCallee, const CXXRecordDecl *RD, - TargetInfo::CallingConvKind CCK, Sema &S) { +static bool computeCanPassInRegister(bool DestroyedInCallee, + const CXXRecordDecl *RD, + TargetInfo::CallingConvKind CCK, + Sema &S) { if (RD->isDependentType() || RD->isInvalidDecl()) - return RecordDecl::APK_CanPassInRegs; + return true; - // The param cannot be passed in registers if ArgPassingRestrictions is set to - // APK_CanNeverPassInRegs. - if (RD->getArgPassingRestrictions() == RecordDecl::APK_CanNeverPassInRegs) - return RecordDecl::APK_CanNeverPassInRegs; + // The param cannot be passed in registers if CanPassInRegisters is already + // set to false. + if (!RD->canPassInRegisters()) + return false; if (CCK != TargetInfo::CCK_MicrosoftX86_64) - return DestroyedInCallee ? RecordDecl::APK_CanPassInRegs - : RecordDecl::APK_CannotPassInRegs; + return DestroyedInCallee; bool CopyCtorIsTrivial = false, CopyCtorIsTrivialForCall = false; bool DtorIsTrivialForCall = false; @@ -5900,7 +5900,7 @@ computeArgPassingRestrictions(bool DestroyedInCallee, const CXXRecordDecl *RD, // If the copy ctor and dtor are both trivial-for-calls, pass direct. if (CopyCtorIsTrivialForCall && DtorIsTrivialForCall) - return RecordDecl::APK_CanPassInRegs; + 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 @@ -5914,8 +5914,8 @@ computeArgPassingRestrictions(bool DestroyedInCallee, const CXXRecordDecl *RD, // passed in registers, which is non-conforming. if (CopyCtorIsTrivial && S.getASTContext().getTypeSize(RD->getTypeForDecl()) <= 64) - return RecordDecl::APK_CanPassInRegs; - return RecordDecl::APK_CannotPassInRegs; + return true; + return false; } /// \brief Perform semantic checks on a class definition that has been @@ -6090,8 +6090,8 @@ void Sema::CheckCompletedCXXClass(CXXRecordDecl *Record) { if (Record->hasNonTrivialDestructor()) Record->setParamDestroyedInCallee(DestroyedInCallee); - Record->setArgPassingRestrictions( - computeArgPassingRestrictions(DestroyedInCallee, Record, CCK, *this)); + Record->setCanPassInRegisters( + computeCanPassInRegister(DestroyedInCallee, Record, CCK, *this)); } /// Look up the special member function that would be called by a special diff --git a/clang/lib/Serialization/ASTReaderDecl.cpp b/clang/lib/Serialization/ASTReaderDecl.cpp index af4cf84..a6cc69d 100644 --- a/clang/lib/Serialization/ASTReaderDecl.cpp +++ b/clang/lib/Serialization/ASTReaderDecl.cpp @@ -742,8 +742,8 @@ ASTDeclReader::VisitRecordDeclImpl(RecordDecl *RD) { RD->setNonTrivialToPrimitiveDefaultInitialize(Record.readInt()); RD->setNonTrivialToPrimitiveCopy(Record.readInt()); RD->setNonTrivialToPrimitiveDestroy(Record.readInt()); + RD->setCanPassInRegisters(Record.readInt()); RD->setParamDestroyedInCallee(Record.readInt()); - RD->setArgPassingRestrictions((RecordDecl::ArgPassingKind)Record.readInt()); return Redecl; } @@ -4114,9 +4114,8 @@ void ASTDeclReader::UpdateDecl(Decl *D, bool HadRealDefinition = OldDD && (OldDD->Definition != RD || !Reader.PendingFakeDefinitionData.count(OldDD)); + RD->setCanPassInRegisters(Record.readInt()); RD->setParamDestroyedInCallee(Record.readInt()); - RD->setArgPassingRestrictions( - (RecordDecl::ArgPassingKind)Record.readInt()); ReadCXXRecordDefinition(RD, /*Update*/true); // Visible update is handled separately. diff --git a/clang/lib/Serialization/ASTWriter.cpp b/clang/lib/Serialization/ASTWriter.cpp index 7f1199f..3369a54 100644 --- a/clang/lib/Serialization/ASTWriter.cpp +++ b/clang/lib/Serialization/ASTWriter.cpp @@ -5193,8 +5193,8 @@ void ASTWriter::WriteDeclUpdatesBlocks(RecordDataImpl &OffsetsRecord) { case UPD_CXX_INSTANTIATED_CLASS_DEFINITION: { auto *RD = cast(D); UpdatedDeclContexts.insert(RD->getPrimaryContext()); + Record.push_back(RD->canPassInRegisters()); Record.push_back(RD->isParamDestroyedInCallee()); - Record.push_back(RD->getArgPassingRestrictions()); Record.AddCXXDefinitionData(RD); Record.AddOffset(WriteDeclContextLexicalBlock( *Context, const_cast(RD))); diff --git a/clang/lib/Serialization/ASTWriterDecl.cpp b/clang/lib/Serialization/ASTWriterDecl.cpp index 189de14..06ea8e7 100644 --- a/clang/lib/Serialization/ASTWriterDecl.cpp +++ b/clang/lib/Serialization/ASTWriterDecl.cpp @@ -469,8 +469,8 @@ void ASTDeclWriter::VisitRecordDecl(RecordDecl *D) { Record.push_back(D->isNonTrivialToPrimitiveDefaultInitialize()); Record.push_back(D->isNonTrivialToPrimitiveCopy()); Record.push_back(D->isNonTrivialToPrimitiveDestroy()); + Record.push_back(D->canPassInRegisters()); Record.push_back(D->isParamDestroyedInCallee()); - Record.push_back(D->getArgPassingRestrictions()); if (D->getDeclContext() == D->getLexicalDeclContext() && !D->hasAttrs() && @@ -1913,10 +1913,9 @@ void ASTWriter::WriteDeclAbbrevs() { Abv->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::Fixed, 1)); // isNonTrivialToPrimitiveDestroy Abv->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::Fixed, 1)); + Abv->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::Fixed, 1)); // canPassInRegisters // isParamDestroyedInCallee Abv->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::Fixed, 1)); - // getArgPassingRestrictions - Abv->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::Fixed, 2)); // DC Abv->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::VBR, 6)); // LexicalOffset diff --git a/clang/test/CodeGenObjCXX/objc-struct-cxx-abi.mm b/clang/test/CodeGenObjCXX/objc-struct-cxx-abi.mm index dd9b88b..de8c5f9 100644 --- a/clang/test/CodeGenObjCXX/objc-struct-cxx-abi.mm +++ b/clang/test/CodeGenObjCXX/objc-struct-cxx-abi.mm @@ -8,8 +8,6 @@ // pointer fields are passed directly. // CHECK: %[[STRUCT_STRONGWEAK:.*]] = type { i8*, i8* } -// CHECK: %[[STRUCT_CONTAINSSTRONGWEAK:.*]] = type { %[[STRUCT_STRONGWEAK]] } -// CHECK: %[[STRUCT_DERIVEDSTRONGWEAK:.*]] = type { %[[STRUCT_STRONGWEAK]] } // CHECK: %[[STRUCT_STRONG:.*]] = type { i8* } // CHECK: %[[STRUCT_S:.*]] = type { i8* } // CHECK: %[[STRUCT_CONTAINSNONTRIVIAL:.*]] = type { %{{.*}}, i8* } @@ -24,21 +22,6 @@ struct StrongWeak { }; #ifdef TRIVIALABI -struct __attribute__((trivial_abi)) ContainsStrongWeak { -#else -struct ContainsStrongWeak { -#endif - StrongWeak sw; -}; - -#ifdef TRIVIALABI -struct __attribute__((trivial_abi)) DerivedStrongWeak : StrongWeak { -#else -struct DerivedStrongWeak : StrongWeak { -#endif -}; - -#ifdef TRIVIALABI struct __attribute__((trivial_abi)) Strong { #else struct Strong { @@ -101,18 +84,6 @@ StrongWeak testReturnStrongWeak(StrongWeak *a) { return *a; } -// CHECK: define void @_Z27testParamContainsStrongWeak18ContainsStrongWeak(%[[STRUCT_CONTAINSSTRONGWEAK]]* %[[A:.*]]) -// CHECK: call %[[STRUCT_CONTAINSSTRONGWEAK]]* @_ZN18ContainsStrongWeakD1Ev(%[[STRUCT_CONTAINSSTRONGWEAK]]* %[[A]]) - -void testParamContainsStrongWeak(ContainsStrongWeak a) { -} - -// CHECK: define void @_Z26testParamDerivedStrongWeak17DerivedStrongWeak(%[[STRUCT_DERIVEDSTRONGWEAK]]* %[[A:.*]]) -// CHECK: call %[[STRUCT_DERIVEDSTRONGWEAK]]* @_ZN17DerivedStrongWeakD1Ev(%[[STRUCT_DERIVEDSTRONGWEAK]]* %[[A]]) - -void testParamDerivedStrongWeak(DerivedStrongWeak a) { -} - // CHECK: define void @_Z15testParamStrong6Strong(i64 %[[A_COERCE:.*]]) // CHECK: %[[A:.*]] = alloca %[[STRUCT_STRONG]], align 8 // CHECK: %[[COERCE_DIVE:.*]] = getelementptr inbounds %[[STRUCT_STRONG]], %[[STRUCT_STRONG]]* %[[A]], i32 0, i32 0 -- 2.7.4