[cfe-dev] Adding a new attribute: no_extern_template

[Louis Dionne via cfe-dev]

> On Aug 21, 2018, at 18:48, Reid Kleckner <rnk at google.com> wrote:
> 
> On Tue, Aug 21, 2018 at 2:44 PM Louis Dionne via cfe-dev <cfe-dev at lists.llvm.org <mailto:cfe-dev at lists.llvm.org>> wrote:
> I've been looking to add an attribute (tentatively called no_extern_template) which would prevent member functions in a template from getting available_externally linkage even when there is a matching extern template declaration. In other words, given the following code:
> 
> The entire point of extern template declarations is to be able to emit member functions with available_externally linkage, so this seems like a pretty strange feature.

Indeed.

> 
> Is the original problem limited to the __init pattern (below), or are is needed for more than that?
>     template <class T>
>     struct Foo {
>         inline __attribute__((visibility("hidden")))
>         int __init(int x) { /* LOTS OF CODE */ }
> 
>         inline __attribute__((visibility("default")))
>         int bar(int x) { return __init(x); }
>     };  
> 
> My first idea (probably too simple to work and already rejected) is to mark bar noinline. Then it will be available_externally, but not inlineable. After CodeGen, we will have a call to the bar definition provided by the libc++ DSO, and there will not be link errors. If that doesn’t work, read on.

I don’t think that’s a good solution for the following reason: We’re currently piggy-backing on __always_inline__ to achieve our goals, and this leads to problems because it’s not really the right tool for the job. Similarly, using `noinline` to achieve our results is just relying on a different (but still wrong) tool. For example, it must be possible to craft cases where using `noinline` leads to suboptimal code being generated. I think inlining should stay out of any solution since that should be for the optimizer to figure out after we’ve given it the information it needs to generate correct code. I could be wrong, of course, but that’s my gut feeling.

> 
> We had a very similar to a problem with dllimport, which also uses available_externally. This is the situation:
> 
> #if BUILDING_FOO
> #define FOO_EXPORT __declspec(dllexport)
> #else
> #define FOO_EXPORT __declspec(dllimport)  
> #endif
> struct Foo {
>   void bar();
>   void FOO_EXPORT foo() { bar(); }
> };
> 
> When compiling the code that uses Foo, BUILDING_FOO is not defined, and dllimport is used. However, Foo::foo calls Foo::bar, which is not exported, and so the link will fail if foo() is emitted available_externally and inlined. In this case, we found that we couldn't actually emit available_externally definitions of foo, we just had to call the out-of-line version provided by the DLL. See the class DLLImportFunctionVisitor in CodeGenModule.cpp that implements this. It’s not bulletproof, but it works well enough.
> 
> I think we might want to consider implementing the same kind of traversal to check for visibility conflicts (a default visibility function calls a hidden one) before emitting available_externally functions for an extern template declaration.

That’s really interesting. I think the problem we’re solving is roughly the same. Do you think I could simply augment the `DLLImportFunctionVisitor` to check for methods that have hidden visibility? Basically, when we’re checking whether `foo` should be emitted, we check whether it has default visibility _and_ if it calls a function with hidden visibility. If that’s the case, we say `foo` should _not_ be emitted, which forces an external call (in the dylib) to be emitted. That would be pretty nice and it would require neither an additional attribute nor any source code changes to libc++ — it would just make visibility(“hidden”) work as one would expect.

It still has the downside of preventing inlining of functions that could otherwise be inlined, but at least that’s not hardcoded in the program’s source code. I’ll try to implement this and see what the problems are, if any.

Louis

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-dev/attachments/20180822/dc7dae91/attachment.html>