<div dir="ltr"><div class="gmail_extra"><div class="gmail_quote"><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex"><span class="">> +  if (D.isFunctionDefinition() && CodeSegStack.CurrentValue &&<br>
<br>
</span>D.isFunctionDefinition() is cheap, but CodeSegStack.CurrentValue is just as<br>
cheap and much more likely to short-circuit; you should check it first.</blockquote><div><br></div><div>Sure</div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex"><div><div class="h5">
> +  if (var->isThisDeclarationADefinition() &&<br>
> +      ActiveTemplateInstantiations.empty()) {<br>
> +    PragmaStack<StringLiteral *> *Stack = nullptr;<br>
> +    int SectionFlags = PSF_Implicit | PSF_Read;<br>
> +    if (var->getType().isConstQualified())<br>
> +      Stack = &ConstSegStack;<br>
> +    else if (!var->getInit()) {<br>
> +      Stack = &BSSSegStack;<br>
> +      SectionFlags |= PSF_Write;<br>
> +    } else {<br>
> +      Stack = &DataSegStack;<br>
> +      SectionFlags |= PSF_Write;<br>
> +    }<br>
> +    if (!var->hasAttr<SectionAttr>() && Stack->CurrentValue)<br>
<br>
</div></div>You should check Stack->CurrentValue first, it’s both cheaper and more likely to short-circuit.<br></blockquote><div><br></div><div>Sure</div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">
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.<br></blockquote><div><br></div><div>I think we want to come in here in !MicrosoftExt mode to do UnifySection. That helps us diagnose this:</div><div><div>__attribute__((section(".mydata"))) extern const int x = 42;</div><div>__attribute__((section(".mydata"))) int y = 42;</div></div><div><div>t.cpp:2:41: error: 'y' causes a section type conflict with 'x'</div></div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">
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.</blockquote><div><br></div><div>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.</div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex"><span class=""><br>
> +      var->addAttr(<br>
> +          SectionAttr::CreateImplicit(Context, SectionAttr::Declspec_allocate,<br>
> +                                      Stack->CurrentValue->getString(),<br>
> +                                      Stack->CurrentPragmaLocation));<br>
> +    if (const SectionAttr *SA = var->getAttr<SectionAttr>())<br>
> +      if (UnifySection(SA->getName(), SectionFlags, var))<br>
> +        var->dropAttr<SectionAttr>();<br>
> +  }<br>
> +<br>
<br>
</span>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.<br></blockquote><div><br></div><div>OK </div></div></div></div>