smeenai added a comment.

In https://reviews.llvm.org/D26657#597523, @hans wrote:

> In https://reviews.llvm.org/D26657#596897, @smeenai wrote:
>
> > In https://reviews.llvm.org/D26657#596759, @hans wrote:
> >
> > > > On MSVC, if an implicit instantiation already exists and an explicit
> > > >  instantiation definition with a DLL attribute is created, the DLL
> > > >  attribute still takes effect. Make clang match this behavior.
> > >
> > > This is scary territory, and behaviour I think might be hard for us to 
> > > match.
> > >
> > > What if we already codegenned the implicit instantiation?
> >
> >
> > Hmm. Under what specific cases would that happen?
>
>
> Actually maybe I was being overly cautius. I think 
> https://reviews.llvm.org/rL225570 might just make this work.
>
> > I can see odd behavior differences occurring for `dllimport`. For example, 
> > for the C++ source in https://reviews.llvm.org/P7936, if I compile with 
> > clang (even with this patch applied), the `t.f()` call in `g` gets 
> > assembled to a call to `?f@?$s@H@@QAEHXZ`. If I hoist the dllimport 
> > explicit instantiation definition above `g`, `t.f()` instead assembles to 
> > `__imp_?f@?$s@H@@QAEHXZ`.
>
> Yeah, dllimport is interesting here. If you turn on `-O1`, it will call the 
> `__imp` one. Fun times :-)


Huh. Weird stuff.

> 
> 
>> With cl 19.00.24210, `t.f()` always assembles to `__imp_?f@?$s@H@@QAEHXZ`, 
>> regardless of the placement of the explicit instantiation definition.
> 
> Presumably they parse and analyze the whole translation unit before emitting 
> any code, which makes changing things around later easier for them than for 
> us.
> 
>> I'm having a harder time imagining things going awry for `dllexport`. I can 
>> limit this patch to only `dllexport`, if that's going to be safer.
> 
> Yeah, I think https://reviews.llvm.org/rL225570 makes this work actually.

So is the conclusion that this is probably safe as is, or should I limit it to 
`dllexport` specifically?

>> To give some context, libc++ uses extern templates extensively, and I ran 
>> into an issue with dllexport explicit locale template instantiations 
>> <https://reviews.llvm.org/diffusion/L/browse/libcxx/trunk/src/locale.cpp;287072$6034-6094?color=1>.
>>  Some of those instantiations are also implicitly instantiated via sizeof 
>> calls 
>> <https://reviews.llvm.org/diffusion/L/browse/libcxx/trunk/src/locale.cpp;287072$67,77,87?color=1>.
>>  Therefore, all instances which make is called on 
>> <https://reviews.llvm.org/diffusion/L/browse/libcxx/trunk/src/locale.cpp;287072$177-204?color=1>
>>  end up having their `dllexport` attribute ignored.
>> 
>> We could work around this in libc++ by hoisting the affected instantiations 
>> to near the top of the file, but it seemed preferable to make clang match 
>> MSVC's behavior instead, at least for `dllexport`. I don't have any 
>> particular interest in making `dllimport` semantics match MSVC for this 
>> specific case.
> 
> Dont' we need `dllimport` to work analogously though? When using libc++, 
> don't these template members need to be dllimported? If not, why are they 
> being exported in the first place?

Ah. We put `dllimport` on the explicit template instantiation declarations in 
the header 
<https://reviews.llvm.org/diffusion/L/browse/libcxx/trunk/include/locale> (all 
the instances of `_LIBCPP_EXTERN_TEMPLATE2` in that file). Those get paired 
with the `dllexport` on the corresponding instantiation definitions. If you're 
compiling the library, the declarations have no DLL attribute, and the 
definitions have `dllexport`. If you're referencing the headers from outside 
the library, the declarations get the `dllimport`.


https://reviews.llvm.org/D26657



_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to