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

Rafael Avila de Espindola via llvm-commits llvm-commits at lists.llvm.org
Thu Mar 23 09:10:04 PDT 2017

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

> 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.

> 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 :-)


More information about the llvm-commits mailing list