[PATCH] Change representation of dllexport/dllimport

[Stephen Lin]

On Wed, Jul 10, 2013 at 6:31 PM, Bill Wendling <wendling at apple.com> wrote:
> On Jul 7, 2013, at 12:35 PM, Reid Kleckner <rnk at google.com> wrote:
>
>> 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.
>>
> We all should have a vested interest in the LLVM IL. :-)
>
> My initial thought would be that adding new linkage types would be the way to go. However, if there are going to be a lot of them, then it's a combinatoric nightmare. If you can mix all other linkage types with the dll* types, then maybe it should be a new field (sub-linkage type?) in the GlobalValue object. Or even expand the size of the linkage field and make it a bit-mask with the dll linkage types.
>
> As for making it an attribute, I'm not against this, but it does seem like a departure from how other linkage types are specified.
>
> I don't know what's best here. But I have a slight preference for the non-attributes way of doing things, because it seems more consistent. So you'll have something like this:
>
>         declare dllexport linkonce_odr void @foo()
>
> Internally, it would be represented like this:
>
> class GlobalValue : public Constant {
> //...
> public:
>   enum LinkageTypes {
> //...
>     LinkeOnceODRLinkage,
> //...
>     DLLImportLinkage = 0x10000,
>     DLLExprtLinkage  = 0x20000
>   };
>
>   unsigned Linkage : 19;
> //...
> };
>
> With equivalent modifications for querying the linkage type.
>
> -bw

FWIW, I would be happy with something like this as well, since
linkages are by their nature more implementation and target-specific.
My general feeling is that a target-independent keyword attribute
should not be added for something which isn't implemented in code
shared across all targets.

Thanks,
Stephen