r332470 - Add support for __declspec(code_seg("segname"))
Richard Smith via cfe-commits
cfe-commits at lists.llvm.org
Fri May 18 13:14:07 PDT 2018
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 <richard at metafoo.co.uk> wrote:
> On 16 May 2018 at 06:57, Erich Keane via cfe-commits <
> cfe-commits at 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 at lists.llvm.org
>> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20180518/44f343c5/attachment-0001.html>
More information about the cfe-commits
mailing list