[LLVMdev] Feedback required on proper dllexport/import implementation

Tue Apr 23 11:16:27 PDT 2013

On Tue, Apr 23, 2013 at 1:10 PM, Gao, Yunzhong <Yunzhong.Gao at am.sony.com> wrote:
> Hi Nico, Reid, and Anton,
>
> I missed the discussion when I implemented dllexport/dllimport for our local tree. I
> essentially implemented your approach#1. I was trying to avoid the various
> external_linkage + some_attribute approaches because it seems that external_linkage
> would imply the external linkage without the dllimport/dllexport semantics, and there
> may be existing compiler codes that rely on this semantics. If I used external_linkage
> + some attribute, then any place in existing codes that uses hasExternalLInkage() will
> have to be modified to check both the linkage type as well as the attribute list, which
> seems a bit fragile to maintain; somebody somewhere will forget to check the attributes
> when writing their new optimization or garbage collection passes.
>
> I think dllimport inline functions are more closely related to the available_externally
> linkage than linkonce_odr; no COMDAT is involved and no .linkonce discard directive
> is needed. I would name it dllimport_inline or something similar.

Ah, you're right.  dllimport_inline is basically available_externally,
with an indirect call through [__imp_foo] if we choose not to inline.

> While it makes sense to mimic MSVC behavior and MSDN doc for C++ codes, it is
> not very clear to me what should be the correct semantics when a C99 inline function
> carries a dllimport/dllexport declspec. For example,
>
> inline __declspec(dllimport) int foo() { return 100; }
> extern inline int foo();
>
> Currently clang ignores the dllimport attribute, so what's left will be:
> inline int foo() { return 100; }
> extern inline int foo(); // this declaration demands an out-of-line definition
>
> The second declaration with the "extern" keyword demands that foo() have an out-
> of-line function definition. If foo() is actually being dllexported from a DLL, there will be
> duplicate definitions at load time (I think some calls will resolve to one symbol and others
> will resolve to a different symbol foo).
>
> If clang wants to respect the dllimport attribute, then the two declarations would have a
> conflict regarding whether a function body should be generated/externally visible.
>
> An alternative is to generate an error message whenever a C99 inline function is used
> with a dllimport/dllexport declspec.

Yep, diagnosing this sounds best.  MSVC doesn't support C99 inline so
there shouldn't be much of this kind of code out there.

> In a similar manner, I wonder what should be the correct semantics of:
> inline __declspec(dllexport) int foo() { return 100; }
>
> I guess I could let dllexport imply "extern", so that an out-of-line definition always gets
> generated, but is that the expected behavior?

Yes, I believe dllexport requires that we instantiate a foo that gets
linked once, and is also exported, like C99 extern inline.

> I built a mingw compiler from this following revision and asked it to compile three C files.
> i386-mingw32-gcc (GCC) 4.9.0 20130416 (experimental)
>
> For the following test case, GCC ignores the dllimport attribute with a warning (similar
> to clang's current behavior)
>
> /* test1.c */
> inline __declspec(dllimport) void foo();
> inline void foo();
> inline void foo() { }
> void foo_user() { foo(); }
> /* end of file */
>
> But for the following two cases, GCC produces both an out-of-line definition and also a
> reference to an dllimported symbol foo with no diagnostics. It seems wrong, unless
> there is some problem with my build process.
>
> /* test2.c */
> inline void foo();
> __declspec(dllimport) void foo();
> inline void foo() { }
> void foo_user() { foo(); }
> /* end of file */
>
> /* test3.c */
> inline void foo();
> __declspec(dllimport) void foo();
> __declspec(dllimport) void foo() { } // clang complains that this is a redeclaration without
>                                                         // "dllimport" attribute", which seems wrong...
> void foo_user() { foo(); }
> /* end of file */
>
> What are your thoughts?
> - Gao.
>
> ________________________________________
> From: llvmdev-bounces at cs.uiuc.edu [llvmdev-bounces at cs.uiuc.edu] on behalf of Nico Rieck [nico.rieck at gmail.com]
> Sent: Monday, March 25, 2013 9:43 PM
> To: LLVM Developers Mailing List
> Subject: [LLVMdev] Feedback required on proper dllexport/import implementation
>
> 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.
>
> 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.
>
>
> -Nico
>
> _______________________________________________
> LLVM Developers mailing list
> LLVMdev at cs.uiuc.edu         http://llvm.cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev
>
>