[LLVMdev] Feedback required on proper dllexport/import implementation

[Gao, Yunzhong]

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.

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.

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?

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