Here's an example regression: https://godbolt.org/g/S72Nki
I'll check that into the test suite alongside the revert once my testing finishes. On 18 May 2018 at 13:03, Richard Smith <rich...@metafoo.co.uk> wrote: > On 16 May 2018 at 06:57, Erich Keane via cfe-commits < > cfe-commits@lists.llvm.org> wrote: > >> Author: erichkeane >> Date: Wed May 16 06:57:17 2018 >> New Revision: 332470 >> >> URL: http://llvm.org/viewvc/llvm-project?rev=332470&view=rev >> Log: >> Add support for __declspec(code_seg("segname")) >> >> Add support for __declspec(code_seg("segname")) >> >> This patch is built on the existing support for #pragma code_seg. The >> code_seg >> declspec is allowed on functions and classes. The attribute enables the >> placement of code into separate named segments, including >> compiler-generated >> members and template instantiations. >> >> For more information, please see the following: >> https://msdn.microsoft.com/en-us/library/dn636922.aspx >> >> A new CodeSeg attribute is used instead of adding a new spelling to the >> existing >> Section attribute since they don’t apply to the same Subjects. Section >> attributes are also added for the code_seg declspec since they are used >> for >> #pragma code_seg. No CodeSeg attributes are added to the AST. >> > > This approach really doesn't work. You're incorrectly applying the > __declspec(code_seg) restrictions to __attribute__((section)), regressing > our support for the latter. Also, code_seg has fundamentally different > semantics from the section attribute -- the former exists in part to > support paged addressing (which is why it has the virtual function > restriction) and the latter exists merely to control properties of the > object file. This approach also violates our policy of maintaining source > fidelity. > > I'm going to revert this for now to fix the regression; when it comes > back, I think you should just use CodeSegAttr to represent > __declspec(code_seg) rather than trying to treat it as -- effectively -- a > different spelling of SectionAttr. > > >> The patch is written to match with the Microsoft compiler’s behavior even >> where >> that behavior is a little complicated (see https://reviews.llvm.org/D2293 >> 1, the >> Microsoft feedback page is no longer available since MS has removed the >> page). >> That code is in getImplicitSectionAttrFromClass routine. >> >> Diagnostics messages are added to match with the Microsoft compiler for >> code-seg >> attribute mismatches on base and derived classes and virtual overrides. >> >> >> Differential Revision: https://reviews.llvm.org/D43352 >> >> Modified: >> cfe/trunk/include/clang/Basic/Attr.td >> cfe/trunk/include/clang/Basic/AttrDocs.td >> cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td >> cfe/trunk/include/clang/Sema/Sema.h >> cfe/trunk/lib/Sema/SemaDecl.cpp >> cfe/trunk/lib/Sema/SemaDeclAttr.cpp >> cfe/trunk/lib/Sema/SemaDeclCXX.cpp >> cfe/trunk/lib/Sema/SemaLambda.cpp >> >> Modified: cfe/trunk/include/clang/Basic/Attr.td >> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/ >> Basic/Attr.td?rev=332470&r1=332469&r2=332470&view=diff >> ============================================================ >> ================== >> --- cfe/trunk/include/clang/Basic/Attr.td (original) >> +++ cfe/trunk/include/clang/Basic/Attr.td Wed May 16 06:57:17 2018 >> @@ -1769,6 +1769,13 @@ def Section : InheritableAttr { >> let Documentation = [SectionDocs]; >> } >> >> +def CodeSeg : InheritableAttr { >> + let Spellings = [Declspec<"code_seg">]; >> + let Args = [StringArgument<"Name">]; >> + let Subjects = SubjectList<[Function, CXXRecord], ErrorDiag>; >> + let Documentation = [CodeSegDocs]; >> +} >> + >> def PragmaClangBSSSection : InheritableAttr { >> // This attribute has no spellings as it is only ever created >> implicitly. >> let Spellings = []; >> >> Modified: cfe/trunk/include/clang/Basic/AttrDocs.td >> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/ >> Basic/AttrDocs.td?rev=332470&r1=332469&r2=332470&view=diff >> ============================================================ >> ================== >> --- cfe/trunk/include/clang/Basic/AttrDocs.td (original) >> +++ cfe/trunk/include/clang/Basic/AttrDocs.td Wed May 16 06:57:17 2018 >> @@ -306,6 +306,18 @@ An example of how to use ``alloc_size`` >> }]; >> } >> >> +def CodeSegDocs : Documentation { >> + let Category = DocCatFunction; >> + let Content = [{ >> +The ``__declspec(code_seg)`` attribute enables the placement of code >> into separate >> +named segments that can be paged or locked in memory individually. This >> attribute >> +is used to control the placement of instantiated templates and >> compiler-generated >> +code. See the documentation for `__declspec(code_seg)`_ on MSDN. >> + >> +.. _`__declspec(code_seg)`: http://msdn.microsoft.com/en-u >> s/library/dn636922.aspx >> + }]; >> +} >> + >> def AllocAlignDocs : Documentation { >> let Category = DocCatFunction; >> let Content = [{ >> >> Modified: cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td >> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/ >> Basic/DiagnosticSemaKinds.td?rev=332470&r1=332469&r2=332470&view=diff >> ============================================================ >> ================== >> --- cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td (original) >> +++ cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td Wed May 16 >> 06:57:17 2018 >> @@ -2668,6 +2668,14 @@ def warn_mismatched_section : Warning< >> def warn_attribute_section_on_redeclaration : Warning< >> "section attribute is specified on redeclared variable">, >> InGroup<Section>; >> >> +def err_mismatched_code_seg_base : Error< >> + "derived class must specify the same code segment as its base >> classes">; >> +def err_mismatched_code_seg_override : Error< >> + "overriding virtual function must specify the same code segment as its >> overridden function">; >> +def err_conflicting_codeseg_attribute : Error< >> + "conflicting code segment specifiers">; >> +def warn_duplicate_codeseg_attribute : Warning< >> + "duplicate code segment specifiers">, InGroup<Section>; >> def err_anonymous_property: Error< >> "anonymous property is not supported">; >> def err_property_is_variably_modified : Error< >> >> Modified: cfe/trunk/include/clang/Sema/Sema.h >> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/ >> Sema/Sema.h?rev=332470&r1=332469&r2=332470&view=diff >> ============================================================ >> ================== >> --- cfe/trunk/include/clang/Sema/Sema.h (original) >> +++ cfe/trunk/include/clang/Sema/Sema.h Wed May 16 06:57:17 2018 >> @@ -1934,6 +1934,7 @@ public: >> bool shouldLinkDependentDeclWithPrevious(Decl *D, Decl *OldDecl); >> void CheckMain(FunctionDecl *FD, const DeclSpec &D); >> void CheckMSVCRTEntryPoint(FunctionDecl *FD); >> + Attr *getImplicitSectionAttrForFunction(const FunctionDecl *FD, bool >> IsDefinition = true); >> Decl *ActOnParamDeclarator(Scope *S, Declarator &D); >> ParmVarDecl *BuildParmVarDeclForTypedef(DeclContext *DC, >> SourceLocation Loc, >> @@ -5836,6 +5837,7 @@ public: >> /// ensure that referenceDLLExportedClassMethods is called some point >> later >> /// when all outer classes of Class are complete. >> void checkClassLevelDLLAttribute(CXXRecordDecl *Class); >> + void checkClassLevelSectionAttribute(CXXRecordDecl *Class); >> >> void referenceDLLExportedClassMethods(); >> >> >> Modified: cfe/trunk/lib/Sema/SemaDecl.cpp >> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaD >> ecl.cpp?rev=332470&r1=332469&r2=332470&view=diff >> ============================================================ >> ================== >> --- cfe/trunk/lib/Sema/SemaDecl.cpp (original) >> +++ cfe/trunk/lib/Sema/SemaDecl.cpp Wed May 16 06:57:17 2018 >> @@ -2667,9 +2667,14 @@ void Sema::mergeDeclAttributes(NamedDecl >> Diag(New->getLocation(), diag::warn_attribute_section_o >> n_redeclaration); >> Diag(Old->getLocation(), diag::note_previous_declaration); >> } >> + } else if (isa<CXXMethodDecl>(New)) { >> + const auto *NewSA = New->getAttr<SectionAttr>(); >> + if (!NewSA->isImplicit()) { >> + Diag(New->getLocation(), diag::warn_mismatched_section); >> + Diag(Old->getLocation(), diag::note_previous_declaration); >> + } >> > > This, in particular, results in bogus warnings for > __attribute__((section)). > > >> } >> } >> - >> if (!Old->hasAttrs()) >> return; >> >> @@ -8716,18 +8721,18 @@ Sema::ActOnFunctionDeclarator(Scope *S, >> >> PragmaClangTextSection.PragmaLocation)); >> } >> >> - // Apply an implicit SectionAttr if #pragma code_seg is active. >> - if (CodeSegStack.CurrentValue && D.isFunctionDefinition() && >> - !NewFD->hasAttr<SectionAttr>()) { >> - NewFD->addAttr( >> - SectionAttr::CreateImplicit(Context, >> SectionAttr::Declspec_allocate, >> - CodeSegStack.CurrentValue->get >> String(), >> - CodeSegStack.CurrentPragmaLoca >> tion)); >> - if (UnifySection(CodeSegStack.CurrentValue->getString(), >> - ASTContext::PSF_Implicit | ASTContext::PSF_Execute | >> - ASTContext::PSF_Read, >> - NewFD)) >> - NewFD->dropAttr<SectionAttr>(); >> + // Apply an implicit SectionAttr from class declspec or from >> + // #pragma code_seg if active. >> + if (!NewFD->hasAttr<SectionAttr>()) { >> + if (Attr *SAttr = getImplicitSectionAttrForFunction(NewFD, >> + >> D.isFunctionDefinition())) { >> + NewFD->addAttr(SAttr); >> + if (UnifySection(cast<SectionAttr>(SAttr)->getName(), >> + ASTContext::PSF_Implicit | >> ASTContext::PSF_Execute | >> + ASTContext::PSF_Read, >> + NewFD)) >> + NewFD->dropAttr<SectionAttr>(); >> + } >> } >> >> // Handle attributes. >> @@ -9177,6 +9182,64 @@ Sema::ActOnFunctionDeclarator(Scope *S, >> return NewFD; >> } >> >> +/// Return a SectionAttr from a containing class. The Microsoft docs say >> +/// when __declspec(code_seg) "is applied to a class, all member >> functions of >> +/// the class and nested classes -- this includes compiler-generated >> special >> +/// member functions -- are put in the specified segment." >> +/// The actual behavior is a little more complicated. The Microsoft >> compiler >> +/// won't check outer classes if there is an active value from #pragma >> code_seg. >> +/// The section is always applied from the direct parent but only from >> outer >> +/// classes when the #pragma code_seg stack is empty. See: >> +/// https://reviews.llvm.org/D22931, the Microsoft feedback page is no >> longer >> +/// available since MS has removed the page. >> +static Attr *getImplicitSectionAttrFromClass(Sema &S, const >> FunctionDecl *FD) { >> + const auto *Method = dyn_cast<CXXMethodDecl>(FD); >> + if (!Method) >> + return nullptr; >> + const CXXRecordDecl *Parent = Method->getParent(); >> + if (const auto *SAttr = Parent->getAttr<SectionAttr>()) { >> + Attr *NewAttr = SAttr->clone(S.getASTContext()); >> + NewAttr->setImplicit(true); >> + return NewAttr; >> + } >> + >> + // The Microsoft compiler won't check outer classes for the section >> + // when the #pragma code_seg stack is active. >> + if (S.CodeSegStack.CurrentValue) >> + return nullptr; >> + >> + while ((Parent = dyn_cast<CXXRecordDecl>(Parent->getParent()))) { >> + if (const auto *SAttr = Parent->getAttr<SectionAttr>()) { >> + Attr *NewAttr = SAttr->clone(S.getASTContext()); >> + NewAttr->setImplicit(true); >> + return NewAttr; >> + } >> + } >> + return nullptr; >> +} >> + >> +/// \brief Returns an implicit SectionAttr for a function. >> +/// >> +/// \param FD Function being declared. >> +/// \param IsDefinition Whether it is a definition or just a >> declarartion. >> +/// \returns A SectionAttr to apply to the function or nullptr if no >> +/// attribute should be added. >> +/// >> +/// First tries to find a SectionAttr on a containing class (from >> +/// a __declspec(code_seg)). If not found on the class, and if the >> function is >> +/// also a definition it will use the current #pragma code_seg value. >> +Attr *Sema::getImplicitSectionAttrForFunction(const FunctionDecl *FD, >> bool IsDefinition) { >> + if (Attr *A = getImplicitSectionAttrFromClass(*this, FD)) >> + return A; >> + if (IsDefinition && CodeSegStack.CurrentValue) { >> + return SectionAttr::CreateImplicit(getASTContext(), >> + SectionAttr::Declspec_allocate, >> + CodeSegStack.CurrentValue->ge >> tString(), >> + CodeSegStack.CurrentPragmaLoc >> ation); >> + } >> + return nullptr; >> +} >> + >> /// Checks if the new declaration declared in dependent context must be >> /// put in the same redeclaration chain as the specified declaration. >> /// >> >> Modified: cfe/trunk/lib/Sema/SemaDeclAttr.cpp >> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaD >> eclAttr.cpp?rev=332470&r1=332469&r2=332470&view=diff >> ============================================================ >> ================== >> --- cfe/trunk/lib/Sema/SemaDeclAttr.cpp (original) >> +++ cfe/trunk/lib/Sema/SemaDeclAttr.cpp Wed May 16 06:57:17 2018 >> @@ -2853,6 +2853,13 @@ static void handleVecTypeHint(Sema &S, D >> SectionAttr *Sema::mergeSectionAttr(Decl *D, SourceRange Range, >> StringRef Name, >> unsigned AttrSpellingListIndex) { >> + // Explicit or partial specializations do not inherit >> + // the code_seg attribute from the primary template. >> + if (const auto *FD = dyn_cast<FunctionDecl>(D)){ >> + if (FD->isFunctionTemplateSpecialization()) >> + return nullptr; >> + } >> + >> if (SectionAttr *ExistingAttr = D->getAttr<SectionAttr>()) { >> if (ExistingAttr->getName() == Name) >> return nullptr; >> @@ -2942,6 +2949,27 @@ static void handleTargetAttr(Sema &S, De >> D->addAttr(NewAttr); >> } >> >> +static void handleCodeSegAttr(Sema &S, Decl *D, const AttributeList &AL) >> { >> + StringRef Str; >> + SourceLocation LiteralLoc; >> + if (!S.checkStringLiteralArgumentAttr(AL, 0, Str, &LiteralLoc)) >> + return; >> + if (!S.checkSectionName(LiteralLoc, Str)) >> + return; >> + if (const auto *ExistingAttr = D->getAttr<SectionAttr>()) { >> + if (!ExistingAttr->isImplicit()) { >> + S.Diag(AL.getLoc(), >> + ExistingAttr->getName() == Str >> + ? diag::warn_duplicate_codeseg_attribute >> + : diag::err_conflicting_codeseg_attribute); >> + return; >> + } >> + D->dropAttr<SectionAttr>(); >> + } >> + D->addAttr(::new (S.Context) SectionAttr( >> + AL.getRange(), S.Context, Str, AL.getAttributeSpellingListInd >> ex())); >> +} >> + >> static void handleCleanupAttr(Sema &S, Decl *D, const AttributeList &AL) >> { >> Expr *E = AL.getArgAsExpr(0); >> SourceLocation Loc = E->getExprLoc(); >> @@ -6297,6 +6325,9 @@ static void ProcessDeclAttribute(Sema &S >> case AttributeList::AT_Uuid: >> handleUuidAttr(S, D, AL); >> break; >> + case AttributeList::AT_CodeSeg: >> + handleCodeSegAttr(S, D, AL); >> + break; >> case AttributeList::AT_MSInheritance: >> handleMSInheritanceAttr(S, D, AL); >> break; >> >> Modified: cfe/trunk/lib/Sema/SemaDeclCXX.cpp >> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaD >> eclCXX.cpp?rev=332470&r1=332469&r2=332470&view=diff >> ============================================================ >> ================== >> --- cfe/trunk/lib/Sema/SemaDeclCXX.cpp (original) >> +++ cfe/trunk/lib/Sema/SemaDeclCXX.cpp Wed May 16 06:57:17 2018 >> @@ -2231,6 +2231,20 @@ Sema::CheckBaseSpecifier(CXXRecordDecl * >> CXXRecordDecl *CXXBaseDecl = cast<CXXRecordDecl>(BaseDecl); >> assert(CXXBaseDecl && "Base type is not a C++ type"); >> >> + // Microsoft docs say: >> + // "If a base-class has a code_seg attribute, derived classes must >> have the >> + // same attribute." >> + const auto *BaseSA = CXXBaseDecl->getAttr<SectionAttr>(); >> + const auto *DerivedSA = Class->getAttr<SectionAttr>(); >> + if (BaseSA || DerivedSA) { >> + if (!BaseSA || !DerivedSA || BaseSA->getName() != >> DerivedSA->getName()) { >> + Diag(Class->getLocation(), diag::err_mismatched_code_seg_base); >> + Diag(CXXBaseDecl->getLocation(), diag::note_base_class_specifie >> d_here) >> + << CXXBaseDecl; >> + return nullptr; >> + } >> + } >> + >> // A class which contains a flexible array member is not suitable for >> use as a >> // base class: >> // - If the layout determines that a base comes before another base, >> @@ -5576,6 +5590,16 @@ static void checkForMultipleExportedDefa >> } >> } >> >> +void Sema::checkClassLevelSectionAttribute(CXXRecordDecl *Class) { >> + // Mark any compiler-generated routines with the implicit Section >> attribute. >> + for (auto *Method : Class->methods()) { >> + if (Method->isUserProvided()) >> + continue; >> + if (Attr *A = getImplicitSectionAttrForFunction(Method)) >> + Method->addAttr(A); >> + } >> +} >> + >> /// Check class-level dllimport/dllexport attribute. >> void Sema::checkClassLevelDLLAttribute(CXXRecordDecl *Class) { >> Attr *ClassAttr = getDLLAttr(Class); >> @@ -6079,6 +6103,7 @@ void Sema::CheckCompletedCXXClass(CXXRec >> } >> >> checkClassLevelDLLAttribute(Record); >> + checkClassLevelSectionAttribute(Record); >> >> bool ClangABICompat4 = >> Context.getLangOpts().getClangABICompat() <= >> LangOptions::ClangABI::Ver4; >> @@ -14531,6 +14556,16 @@ bool Sema::CheckOverridingFunctionAttrib >> diag::note_overridden_marked_noescape); >> } >> } >> + // Virtual overrides must have the same code_seg. >> + const auto *OldSA = Old->getAttr<SectionAttr>(); >> + const auto *NewSA = New->getAttr<SectionAttr>(); >> + if (OldSA || NewSA) { >> + if (!OldSA || !NewSA || NewSA->getName() != OldSA->getName()) { >> + Diag(New->getLocation(), diag::err_mismatched_code_seg_override); >> + Diag(Old->getLocation(), diag::note_previous_declaration); >> + return true; >> + } >> + } >> >> CallingConv NewCC = NewFT->getCallConv(), OldCC = OldFT->getCallConv(); >> >> >> Modified: cfe/trunk/lib/Sema/SemaLambda.cpp >> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaL >> ambda.cpp?rev=332470&r1=332469&r2=332470&view=diff >> ============================================================ >> ================== >> --- cfe/trunk/lib/Sema/SemaLambda.cpp (original) >> +++ cfe/trunk/lib/Sema/SemaLambda.cpp Wed May 16 06:57:17 2018 >> @@ -910,6 +910,10 @@ void Sema::ActOnStartOfLambdaDefinition( >> AddRangeBasedOptnone(Method); >> >> // Attributes on the lambda apply to the method. >> + if (Attr *A = getImplicitSectionAttrForFunction(Method)) >> + Method->addAttr(A); >> + >> + // Attributes on the lambda apply to the method. >> ProcessDeclAttributes(CurScope, Method, ParamInfo); >> >> // CUDA lambdas get implicit attributes based on the scope in which >> they're >> >> >> _______________________________________________ >> cfe-commits mailing list >> cfe-commits@lists.llvm.org >> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits >> > >
_______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
