r205810 - [MS-ABI] Add support for #pragma section and related pragmas

[Reid Kleckner]

Should be addressed in r231317.

On Wed, Mar 4, 2015 at 3:30 PM, Reid Kleckner <rnk at google.com> wrote:

> > +  if (D.isFunctionDefinition() && CodeSegStack.CurrentValue &&
>>
>> D.isFunctionDefinition() is cheap, but CodeSegStack.CurrentValue is just
>> as
>> cheap and much more likely to short-circuit; you should check it first.
>
>
> Sure
>
>> > +  if (var->isThisDeclarationADefinition() &&
>> > +      ActiveTemplateInstantiations.empty()) {
>> > +    PragmaStack<StringLiteral *> *Stack = nullptr;
>> > +    int SectionFlags = PSF_Implicit | PSF_Read;
>> > +    if (var->getType().isConstQualified())
>> > +      Stack = &ConstSegStack;
>> > +    else if (!var->getInit()) {
>> > +      Stack = &BSSSegStack;
>> > +      SectionFlags |= PSF_Write;
>> > +    } else {
>> > +      Stack = &DataSegStack;
>> > +      SectionFlags |= PSF_Write;
>> > +    }
>> > +    if (!var->hasAttr<SectionAttr>() && Stack->CurrentValue)
>>
>> You should check Stack->CurrentValue first, it’s both cheaper and more
>> likely to short-circuit.
>>
>
> Sure
>
>
>> In fact, you should consider just keeping a flag on Sema about whether
>> *any* data section pragmas are active, since I assume that none are for the
>> vast majority of time, and it would be good to avoid this entire block in
>> that case.  If you don’t do that, you should at least guard the block on
>> getLangOpts().MicrosoftExt.
>>
>
> I think we want to come in here in !MicrosoftExt mode to do UnifySection.
> That helps us diagnose this:
> __attribute__((section(".mydata"))) extern const int x = 42;
> __attribute__((section(".mydata"))) int y = 42;
> t.cpp:2:41: error: 'y' causes a section type conflict with 'x'
>
>
>> You should also not apply a section attribute to a variable of automatic
>> storage duration, and you should check with MSVC about whether they apply
>> to local statics and static member variables, if you haven’t already.
>
>
> MSVC applies these pragmas to static locals and static data members, but
> not locals obviously. I can short-circuit out of the stack computation for
> local variables.
>
>
>>
>> > +      var->addAttr(
>> > +          SectionAttr::CreateImplicit(Context,
>> SectionAttr::Declspec_allocate,
>> > +                                      Stack->CurrentValue->getString(),
>> > +                                      Stack->CurrentPragmaLocation));
>> > +    if (const SectionAttr *SA = var->getAttr<SectionAttr>())
>> > +      if (UnifySection(SA->getName(), SectionFlags, var))
>> > +        var->dropAttr<SectionAttr>();
>> > +  }
>> > +
>>
>> Please ask the target to validate the section name with
>> TargetInfo::isValidSectionSpecifier so that a malformed section doesn't
>> simply crash the backend.  It’s probably best to do that at the point of
>> the pragma.
>>
>
> OK
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20150304/bf7b6c7a/attachment.html>