[LLVMdev] Feedback required on proper dllexport/import implementation

[Reid Kleckner]

On Mon, Mar 25, 2013 at 9:43 PM, Nico Rieck <nico.rieck at gmail.com> wrote:

> Hello,
>
> while improving and extending support for dllexport/import I have
> noticed that the current way these are implemented is problematic and I
> would like some input on how to proceed.
>
> Currently dllexport/dllimport is treated as linkage type. This conflicts
> with inlined functions because there is no linkage for the combination
> of both. On first though, combining both doesn't make sense, but take
> this class as an example:
>
>   struct __declspec(dllexport/dllimport) X {
>     X() {}
>   };
>
> Such constructs with empty or simple inline functions can be found for
> example in Microsoft headers or even libc++ headers.
>
> Ignoring the dllexport/import attribute for such functions would seem
> most sensible (and can be implemented easily), but MSVC does the
> opposite: For imported inline functions the function body is dropped and
> __imp_ calls are emitted, and exported inline functions are placed into
> COMDAT sections. The latter cannot be expressed because it requires
> linkonce_odr and dllexport linkage.
>

Hang on, to me the docs seem to say the dllimport case is slightly
different:
http://msdn.microsoft.com/en-us/library/xa0d9ste.aspx

dllexport: May be inlined, but always gets instantiated as COMDAT and
ultimately gets exported after linking.

dllimport: May be inlined.  If inlining fails, an import-style call
(__imp_*) is emitted.

Basically, it avoids COMDAT cruft when we can be sure a definition will be
provided by some imported dll.  That feels like a quality of implementation
optimization.  I suppose someone could be using /Ob0 or -fno-inline to
ignore the definition from the header file and always get the import, but
that seems like a corner case.


> The question now is how to implement this. After a brief discussion, I
> can think of four ways:
>
> 1. Add additional linkage type(s) for the combinations to
>    GlobalValue::LinkageTypes.
>
>    This appears to be the least invasive way, but adds new linkage types
>    to an already large list.
>
> 2. Handle dllexport/import similar to ELF visibility by adding new
>    "visibility" types to GlobalValue::VisibilityTypes and IR visibility
>    styles.
>
>    This feels like kind of a band-aid. While dllexport could be
>    construed as similar to default visibility (some code uses both in
>    the same place depending on platform), dllimport feels wrong here.
>    This would also prevent mixing ELF visibility with dllexport/import.
>
>    The size of GlobalValue can be kept the same by simply adjusting the
>    bit-fields for linkage and visibility.
>
> 3. Add a new enum for dllimport/export to GlobalValue and IR global
>    variables and functions, similar to ELF visibility.
>
>    This is similar to (2.), without the awkward piggybacking on
>    visibility. But it requires extensions to IR just for a single
>    platform.
>
>    The size of GlobalValue can be kept the same by simply adjusting the
>    bit-fields.
>
> 4. Handle dllexport/import as platform-specific IR function attributes.
>
>    Because dllexport/import can also apply to globals which have no
>    attributes, this requires keeping the dllexport/import linkage types
>    just for them.
>
>    Inline does not apply to globals, but MSVC can actually produce
>    initialized dllexported globals, placed in COMDAT sections, with
>    __declspec(selectany). I have no idea if anyone actually does this.
>    LLVM also does not support __declspec(selectany) yet.
>
> There may be even more or better ways to implement this. It may be good
> to keep a future implementation of __declspec(selectany) in mind when
> thinking about this issue.
>

I'm not super familiar with this code, but I think approach 1 is most
consistent with the existing code.  There are already a handful of linkage
types that represent combinations of properties (weak, odr, external,
internal, etc).  It kind of limits the number of linkage combinations that
LLVM needs to think about or support.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20130326/39c2ead1/attachment.html>