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

[Reid Kleckner]

>
> > +  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/f57dcc0a/attachment.html>