[PATCH] Change representation of dllexport/dllimport

[Reid Kleckner]

Thanks for working on this!

Some links to previous discussion for anyone else following along.
The original design RFC on llvmdev:
http://lists.cs.uiuc.edu/pipermail/llvmdev/2013-March/060646.html
The combined clang/llvm patch with a more detailed CL description:
http://llvm-reviews.chandlerc.com/D1099

Phab is down, so I'll just make some high level comments.

>From the original 4 approaches, it looks like you went with #4 (new
attributes).  Adding new visibilities (#3) is still on the table if people
don't like the duplication between global variables and functions.  I'd
like to hear from someone with more of a vested interest in the LLVM IL.

In the original RFC, you suggested that someone might want to mix
visibility and dll export control.  Is that a real use case?  I don't think
PE DLLs can actually implement ELF's simple, C-like global namespace, so it
sounds like you want to be able to emit ELF files using dllexport.

In that case, I don't think there are any valid combinations of visibility
+ dllexport.  dllexport is closest to visibility(default), plus weak_odr
instead of linkonce_odr for inline functions.  dllimport, as previously
mentioned, is closer to available_externally with some tweaks to use
*__imp_foo.  Finally, GCC's docs say that dllexport implies visibility
default
http://gcc.gnu.org/onlinedocs/gcc/Function-Attributes.html#index-g_t_0040code_007b_005f_005fdeclspec_0028dllexport_0029_007d-2630,
meaning you can't combine them with gcc.

At this point, I've convinced myself that the export control stuff is best
modeled as a new visibility.

Regardless of attributes vs. visibilities, you shouldn't break the C API.
 The C++ API is always in flux, but the C API is stable.  You can't remove
the linkage enums from llvm-c/Core.h, or at least you should keep the
subsequent values stable.  See LLVMGhostLinkage for prior art.

You should also auto-upgrade decoded bitcode.  Make GetDecodedLinkage()
return ExternalLinkage for the old codes for both dllexport and dllimport
linkages, and then add another helper to apply the correct attribute or
visibility.

If you do end up changing the grammar for global variables, you should add
a "Syntax:" block to LangRef for globals like there is for most other
language constructs.


On Fri, Jul 5, 2013 at 9:38 PM, Nico Rieck <nico.rieck at gmail.com> wrote:

> Representing dllexport/dllimport as distinct linkage types prevents using
> these attributes on templates and inline functions.
>
> Instead of introducing further mixed linkage types to include linkonce and
> weak ODR, the old import/export linkage types are replaced with new
> function attributes (DLLExport, DLLImport) and an extension to global
> variables:
>
>       @Exp = global i32 1, align 4, dllexport
>       @Imp = external global i32, dllimport
>
> Linkage for dllexported globals and functions is now equal to their
> linkage without dllexport. Imported globals and functions must be either
> declarations, or definitions with AvailableExternallyLinkage. An alias is
> exported if the aliasee is exported.
>
> http://llvm-reviews.chandlerc.com/D1110
>
> Files:
>   docs/LangRef.rst
>   include/llvm-c/Core.h
>   include/llvm/IR/Attributes.h
>   include/llvm/IR/GlobalValue.h
>   include/llvm/IR/GlobalVariable.h
>   lib/AsmParser/LLParser.cpp
>   lib/Bitcode/Reader/BitcodeReader.cpp
>   lib/Bitcode/Writer/BitcodeWriter.cpp
>   lib/CodeGen/AsmPrinter/AsmPrinter.cpp
>   lib/ExecutionEngine/ExecutionEngine.cpp
>   lib/IR/AsmWriter.cpp
>   lib/IR/Attributes.cpp
>   lib/IR/Core.cpp
>   lib/IR/Globals.cpp
>   lib/IR/Verifier.cpp
>   lib/Linker/LinkModules.cpp
>   lib/Target/CppBackend/CPPBackend.cpp
>   lib/Target/Mangler.cpp
>   lib/Target/X86/X86AsmPrinter.cpp
>   lib/Target/X86/X86FastISel.cpp
>   lib/Target/X86/X86ISelLowering.cpp
>   lib/Target/X86/X86Subtarget.cpp
>   lib/Target/XCore/XCoreAsmPrinter.cpp
>   test/CodeGen/X86/dll-linkage.ll
>   test/CodeGen/X86/dllexport-x86_64.ll
>   test/CodeGen/X86/dllexport.ll
>   test/CodeGen/X86/dllimport-x86_64.ll
>   test/CodeGen/X86/dllimport.ll
>   test/MC/COFF/linker-options.ll
>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20130707/64ac0149/attachment.html>