[PATCH] D31270: [wip] IR: Move linker options to top-level global metadata and remove dllexport storage class.

[Peter Collingbourne via llvm-commits]

On Thu, Mar 23, 2017 at 9:10 AM, Rafael Avila de Espindola <
rafael.espindola at gmail.com> wrote:

> Peter Collingbourne via Phabricator <reviews at reviews.llvm.org> writes:
>
> > pcc created this revision.
> > Herald added a subscriber: mehdi_amini.
> >
> > dllexport is not actually a global-level attribute. At the object file
> > level, it is in fact a module-level attribute and is represented with an
> > /export flag in the directive section. Trying to shoehorn it into being a
> > global-level attribute creates unnecessary complexity and a burden on the
> > rest of the compiler and invites bugs.
>
> But that directive is a list, so in this regard they seem equivalent.
>
> In PR32334 I see:
>
> ------------------------------------------------------------
> In COFF, exports are identified by a list of /export flags in the
> .drective section, so when a comdat section is discarded, we don't lose
> the information that the symbol might be exported.
> -----------------------------------------------------------------
>
> Which is the case where the two representations are different. Even if
> we drop the last IR @foo, we still need to remember that @foo is to be
> exported.


Indeed. I will update the commit message to make that clearer.

> Independent of whether it is a global- or module-level attribute, it makes
> > sense to split dllimport from dllexport, as they are somewhat
> orthogonal. The
> > meaning of dllexport is similar to ELF symbol visibility, while the
> meaning
> > of dllimport is closer to preemptability (see also
> > https://reviews.llvm.org/D20217).
>
> I agree. dllexport is almost redundant. Using ELF terminology we can say
> that COFF systems just default to hidden visibility and have the option
> of using protected when they want to export a symbol. The analogy is not
> exact in that ELF visibilities are merged by taking the minimum.
>

Right. In principle we have an analogous situation in ELF: the IR may
contain a declaration such as:

declare hidden void @foo()

and that could have an effect on foo's visibility (assuming it is >hidden)
depending on whether the foo declaration was dropped.

I think the solution is to think of a non-defining declaration's visibility
as an ODR-relevant property (i.e. if you specify a non-default visibility,
it has to agree with the prevailing definition). As PR32334 shows,
dllexport cannot be relied on as an ODR property in COFF.

> With this,
> > dllimport can potentially receive a meaning in other object formats
> without
> > being in the confusing situation where dllexport only has meaning for
> COFF
> > and dllimport also has meaning elsewhere.
>
> I would love to use dllimport on ELF to mean "assume this symbol is
> external regardless of -fPIC".
>
> > This patch does three things:
> >
> > - Moves the linker options from a module flag to top-level module
> metadata. Storing it as a module flag only makes it harder to manipulate
> the linker options.
> > - Causes clang to add an /export flag to the linker options metadata if
> it is emitting a definition that will be dllexported.
> > - Removes the dllexport storage class and stores dllimport as a boolean.
>
> I love this change :-)
>
>
> Cheers,
> Rafael
>



-- 
-- 
Peter
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20170323/40e6574b/attachment.html>