[PATCH] D29233: Fix linkage of static locals in available_externally functions to be DiscardableODR/linkonce_odr

David Blaikie via cfe-commits cfe-commits at lists.llvm.org
Fri Jan 27 14:50:02 PST 2017


On Fri, Jan 27, 2017 at 2:48 PM Mehdi Amini <mehdi.amini at apple.com> wrote:

> On Jan 27, 2017, at 2:44 PM, David Blaikie <dblaikie at gmail.com> wrote:
>
>
>
> On Fri, Jan 27, 2017 at 2:11 PM Mehdi AMINI via Phabricator <
> reviews at reviews.llvm.org> wrote:
>
> mehdi_amini accepted this revision.
> mehdi_amini added a comment.
> This revision is now accepted and ready to land.
>
> LGTM.
>
>
>
> ================
> Comment at: lib/AST/ASTContext.cpp:8909
> +
> +    // Itanium ABI (& MSVC seems to do similarly) requires static locals
> in
> +    // inline functions to be emitted anywhere they're needed, even if the
> ----------------
> I assume you looked it up, do you have a ref? (Citation or pointer to
> right section/paragraph).
>
>
> Would you like a citation in source?
>
>
> Sure, why not?
>
> I thought that might be a bit strong since this is ABI-neutral code,
> notionally (so I was straddling that line a bit).
>
>
> Well you refer to the ABI anyway already to explain the chosen behavior :)
>
> And technically I’m not sure what the C++ standard mandates, the
> possibility of having different need for different ABI would indicate that
> the code can’t be totally ABI-neutral?
>

Right, this one is beyond the C++ standard's reach. It's really an ABI
issue, so arguably should be sunk into Clang's ABI understanding, just that
for now all (Itanium + MSVC) the situations we know of/support have this
behavior so there's no immediate need for that abstraction/extension point.


>
>> Mehdi
>
>
>
> I can't quite figure out how to navigate/find the ABI document nor cast
> the runes as Richard did for the citation, so hopefully he can chime in
> here.
>
>
>
>
> https://reviews.llvm.org/D29233
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20170127/b660475d/attachment.html>


More information about the cfe-commits mailing list