<html><head><meta http-equiv="Content-Type" content="text/html; charset=utf-8"></head><body style="word-wrap: break-word; -webkit-nbsp-mode: space; line-break: after-white-space;" class=""><br class=""><div><br class=""><blockquote type="cite" class=""><div class="">On Aug 22, 2018, at 11:32, Louis Dionne via cfe-dev <<a href="mailto:cfe-dev@lists.llvm.org" class="">cfe-dev@lists.llvm.org</a>> wrote:</div><br class="Apple-interchange-newline"><div class=""><div style="caret-color: rgb(0, 0, 0); font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; text-decoration: none;" class=""><br class="Apple-interchange-newline"><br class=""><blockquote type="cite" class=""><div class="">On Aug 21, 2018, at 18:48, Reid Kleckner <<a href="mailto:rnk@google.com" class="">rnk@google.com</a>> wrote:</div><br class="Apple-interchange-newline"><div class=""><div dir="ltr" class=""><div class="gmail_quote"><div dir="ltr" class="">On Tue, Aug 21, 2018 at 2:44 PM Louis Dionne via cfe-dev <<a href="mailto:cfe-dev@lists.llvm.org" target="_blank" class="">cfe-dev@lists.llvm.org</a>> wrote:<br class=""></div><blockquote class="gmail_quote" style="margin: 0px 0px 0px 0.8ex; border-left-width: 1px; border-left-style: solid; border-left-color: rgb(204, 204, 204); padding-left: 1ex;">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:<br class=""></blockquote><div class=""><br class=""></div><div class="">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.<br class=""></div></div></div></div></blockquote><div class=""><br class=""></div><div class="">Indeed.</div><br class=""><blockquote type="cite" class=""><div class=""><div dir="ltr" class=""><div class="gmail_quote"><div class=""><br class=""></div><div class="">Is the original problem limited to the __init pattern (below), or are is needed for more than that?</div><div class=""> template <class T><br class=""> struct Foo {<br class=""> inline __attribute__((visibility("hidden")))<br class=""> int __init(int x) { /* LOTS OF CODE */ }<br class=""><br class=""> inline __attribute__((visibility("default")))<br class=""> int bar(int x) { return __init(x); }<br class=""> }; <br class=""></div><div class=""><br class=""></div><div class="">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.</div></div></div></div></blockquote><div class=""><br class=""></div><div class="">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.</div><br class=""><blockquote type="cite" class=""><div class=""><div dir="ltr" class=""><div class="gmail_quote"><div class=""><br class=""></div><div class="">We had a very similar to a problem with dllimport, which also uses available_externally. This is the situation:</div><div class=""><br class=""></div><div class="">#if BUILDING_FOO</div><div class="">#define FOO_EXPORT __declspec(dllexport)</div><div class="">#else</div><div class="">#define FOO_EXPORT __declspec(dllimport) <br class=""></div><div class="">#endif</div><div class="">struct Foo {</div><div class=""> void bar();</div><div class=""> void FOO_EXPORT foo() { bar(); }<br class="">};</div><div class=""><br class=""></div><div class="">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.</div></div></div></div></blockquote><blockquote type="cite" class=""><div class=""><div dir="ltr" class=""><div class="gmail_quote"><div class=""><br class=""></div><div class="">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.</div></div></div></div></blockquote><br class=""></div><div style="caret-color: rgb(0, 0, 0); font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; text-decoration: none;" class="">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.</div><div style="caret-color: rgb(0, 0, 0); font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; text-decoration: none;" class=""><br class=""></div><div style="caret-color: rgb(0, 0, 0); font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; text-decoration: none;" class="">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.</div></div></blockquote><div><br class=""></div><div><div>I gave some more thought to this approach and implemented a proof of concept, and I think it doesn’t solve my problem. Indeed, if a user calls a function with hidden visibility that happens to be part of an extern template declaration, I still get a link error. IOW, your approach doesn’t work if there’s no function with default visibility somewhere in the chain. This happens for example in std::vector.</div><div><br class=""></div><div>In vector.hpp:</div><div><br class=""></div><div> namespace std {</div><div> template <bool></div><div> class __vector_base_common {</div><div> public:</div><div> __attribute__((visibility("hidden"))) __vector_base_common() {}</div><div> };</div><div><br class=""></div><div> extern template class __attribute__ ((__visibility__("default"))) __vector_base_common<true>;</div><div><br class=""></div><div> template <class _Tp></div><div> class __attribute__ ((__type_visibility__("default"))) vector</div><div> : private __vector_base_common<true></div><div> {</div><div> public:</div><div> __attribute__((__visibility__("hidden"))) vector() { }</div><div> };</div><div> }</div><div><br class=""></div><div>In vector.cpp (which becomes part of libc++.dylib):</div><div><br class=""></div><div> #include "vector.hpp"</div><div> namespace std {</div><div> template class __vector_base_common<true>;</div><div> }</div><div><br class=""></div><div>In main.cpp:</div><div><br class=""></div><div> #include "vector.hpp"</div><div> int main() {</div><div> std::vector<int> v;</div><div> }</div><div><br class=""></div><div>Then, build as:</div><div><br class=""></div><div>$ clang++ -std=c++11 -I . vector.cpp -o libcplusplus.dylib -shared</div><div>$ clang++ -std=c++11 -I . main.cpp -o main.exe -lcplusplus -L .</div><div><br class=""></div><div>std::__vector_base_common<true>::__vector_base_common()", referenced from:</div><div> std::vector<int>::vector() in main-1c9480.o</div><div><br class=""></div><div>The problem here is that there's no function in the dylib that we can call that would itself have access to __vector_base_common<true>::__vector_base_common(), which has hidden visibility.</div><div><br class=""></div><div>Also, even disregarding this issue, it turns out that preventing inlining of all methods that are part of the ABI is kind of a big deal -- I'm quite concerned about the performance impact this could have. I’d much rather pursue a path that does not require telling the compiler whether to inline something or not, as this is an orthogonal decision IMO. Does that make sense?</div><div><br class=""></div><div>Louis</div><div class=""><br class=""></div></div><br class=""><blockquote type="cite" class=""><div class=""><div style="caret-color: rgb(0, 0, 0); font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; text-decoration: none;" class=""><br class=""></div><div style="caret-color: rgb(0, 0, 0); font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; text-decoration: none;" class="">Louis</div><div style="caret-color: rgb(0, 0, 0); font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; text-decoration: none;" class=""><br class=""></div><span style="caret-color: rgb(0, 0, 0); font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; text-decoration: none; float: none; display: inline !important;" class="">_______________________________________________</span><br style="caret-color: rgb(0, 0, 0); font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; text-decoration: none;" class=""><span style="caret-color: rgb(0, 0, 0); font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; text-decoration: none; float: none; display: inline !important;" class="">cfe-dev mailing list</span><br style="caret-color: rgb(0, 0, 0); font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; text-decoration: none;" class=""><a href="mailto:cfe-dev@lists.llvm.org" style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-size-adjust: auto; -webkit-text-stroke-width: 0px;" class="">cfe-dev@lists.llvm.org</a><br style="caret-color: rgb(0, 0, 0); font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; text-decoration: none;" class=""><a href="http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev" style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-size-adjust: auto; -webkit-text-stroke-width: 0px;" class="">http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev</a></div></blockquote></div><br class=""></body></html>