<div dir="ltr">On Mon, Mar 25, 2013 at 9:43 PM, Nico Rieck <span dir="ltr"><<a href="mailto:nico.rieck@gmail.com" target="_blank">nico.rieck@gmail.com</a>></span> wrote:<br><div class="gmail_extra"><div class="gmail_quote">
<blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">Hello,<br>
<br>
while improving and extending support for dllexport/import I have<br>
noticed that the current way these are implemented is problematic and I<br>
would like some input on how to proceed.<br>
<br>
Currently dllexport/dllimport is treated as linkage type. This conflicts<br>
with inlined functions because there is no linkage for the combination<br>
of both. On first though, combining both doesn't make sense, but take<br>
this class as an example:<br>
<br>
  struct __declspec(dllexport/dllimport) X {<br>
    X() {}<br>
  };<br>
<br>
Such constructs with empty or simple inline functions can be found for<br>
example in Microsoft headers or even libc++ headers.<br>
<br>
Ignoring the dllexport/import attribute for such functions would seem<br>
most sensible (and can be implemented easily), but MSVC does the<br>
opposite: For imported inline functions the function body is dropped and<br>
__imp_ calls are emitted, and exported inline functions are placed into<br>
COMDAT sections. The latter cannot be expressed because it requires<br>
linkonce_odr and dllexport linkage.<br></blockquote><div><br></div><div style>Hang on, to me the docs seem to say the dllimport case is slightly different:</div><div style><a href="http://msdn.microsoft.com/en-us/library/xa0d9ste.aspx">http://msdn.microsoft.com/en-us/library/xa0d9ste.aspx</a></div>
<div style><br></div><div style>dllexport: May be inlined, but always gets instantiated as COMDAT and ultimately gets exported after linking.</div><div style><br></div><div style>dllimport: May be inlined.  If inlining fails, an import-style call (__imp_*) is emitted.</div>
<div style><br></div><div style>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.</div>
<div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">
The question now is how to implement this. After a brief discussion, I<br>
can think of four ways:<br>
<br>
1. Add additional linkage type(s) for the combinations to<br>
   GlobalValue::LinkageTypes.<br>
<br>
   This appears to be the least invasive way, but adds new linkage types<br>
   to an already large list.<br>
<br>
2. Handle dllexport/import similar to ELF visibility by adding new<br>
   "visibility" types to GlobalValue::VisibilityTypes and IR visibility<br>
   styles.<br>
<br>
   This feels like kind of a band-aid. While dllexport could be<br>
   construed as similar to default visibility (some code uses both in<br>
   the same place depending on platform), dllimport feels wrong here.<br>
   This would also prevent mixing ELF visibility with dllexport/import.<br>
<br>
   The size of GlobalValue can be kept the same by simply adjusting the<br>
   bit-fields for linkage and visibility.<br>
<br>
3. Add a new enum for dllimport/export to GlobalValue and IR global<br>
   variables and functions, similar to ELF visibility.<br>
<br>
   This is similar to (2.), without the awkward piggybacking on<br>
   visibility. But it requires extensions to IR just for a single<br>
   platform.<br>
<br>
   The size of GlobalValue can be kept the same by simply adjusting the<br>
   bit-fields.<br>
<br>
4. Handle dllexport/import as platform-specific IR function attributes.<br>
<br>
   Because dllexport/import can also apply to globals which have no<br>
   attributes, this requires keeping the dllexport/import linkage types<br>
   just for them.<br>
<br>
   Inline does not apply to globals, but MSVC can actually produce<br>
   initialized dllexported globals, placed in COMDAT sections, with<br>
   __declspec(selectany). I have no idea if anyone actually does this.<br>
   LLVM also does not support __declspec(selectany) yet.<br>
<br>
There may be even more or better ways to implement this. It may be good<br>
to keep a future implementation of __declspec(selectany) in mind when<br>
thinking about this issue.<br></blockquote><div><br></div><div style>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. </div>
</div></div></div>