<div dir="ltr"><div><div>I'm concerned that if you don't mark the vtable used enough, then codegen will crash trying to emit a vtable for a class that it assumed would be marked used because it's compiling a virtual call to a method of such a class. However, it looks like Rafael completely nuked the available_externally vtable emission optimization in r189852, which I forgot about. The vtable code still has lots of rigging to allow available_externally vtable emission, though.</div></div><div><br></div><div>So, I want to say that before r189852, these MarkVTableUsed calls were necessary, and now that we don't do it, they are not. However, I'd really like that optimization back, so we can optimize this code, once again:</div><div><br></div><div><div>struct A {</div><div> virtual void f();</div><div>};</div><div>static void h(A &a) {</div><div> a.f(); // requires inlining, must devirtualize in LLVM, requires available_externally vtable</div><div>}</div><div>void g() {</div><div> A a;</div><div> a.f(); // obvious, devirtualized in frontend</div><div> h(a);</div><div>}</div></div><div></div></div><div class="gmail_extra"><br><div class="gmail_quote">On Tue, Jan 13, 2015 at 10:40 AM, Nico Weber <span dir="ltr"><<a href="mailto:thakis@chromium.org" target="_blank">thakis@chromium.org</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr">Hi,<div><br></div><div>clang currently calls MarkVTableUsed() for classes that get their virtual methods called or that participate in a dynamic_cast. This is unnecessary, since CodeGen only emits vtables when it generates constructor, destructor, and vtt code.</div><div><br></div><div>Remember that sema's MarkVTableUsed() doesn't actually cause emission of a vtable, it only marks all methods that are in the vtable as referenced – to make sure that when codegen decides to write a vtable, everything in there is actually defined. Since virtual calls and dynamic casts do not cause codegen to write a vtable, it's not necessary to define methods in the vtable for them. After this patch, sema only calls MarkVTableUsed() for the definitions of constructors and destructors – the only two things that make codegen write a vtable* (look for `getAddrOfVTable` in lib/CodeGen).</div>
<div><br></div><div>They still get defined if they're referenced by a direct call – this only affects virtual methods that are not called and were only referenced by MarkVTableUsed(), for example if a different virtual method was called). For the same reason, this doesn't have an effect on what llvm can or can't optimize (see also PR20337 comment 10). It does lead to the emission of fewer unneeded methods, which will speed up builds.</div><div><br></div><div>While this shouldn't change the behavior of codegen (other than being faster), it does make clang more permissive: virtual methods (in particular destructors) end up being instantiated less often. In particular, classes that have members that are smart pointers to incomplete types will now get their implicit virtual destructor instantiated less frequently. For example, this used to not compile but does now compile:</div><div><br></div><div><div> template <typename T> struct OwnPtr {</div><div> ~OwnPtr() { static_assert((sizeof(T) > 0), "TypeMustBeComplete"); }</div><div> };</div><div> class ScriptLoader;<br></div><div> struct Base { virtual ~Base(); };<br></div><div> struct Sub : public Base {</div><div> virtual void someFun() const {}</div><div> OwnPtr<ScriptLoader> m_loader;</div><div> };</div><div> void f(Sub *s) { s->someFun(); }<br></div></div><div><br></div><div>The more permissive behavior matches both gcc (where this is not often observable, since in practice most things with virtual methods have a key function) and cl (which is my motivation for this change) – this fixes PR20337.</div><div><br></div><div>This is similar to r213109 in spirit. r225761 was a prerequisite for this change.</div><div><br>Various tests relied on "a->f()" marking a's vtable as used (in the sema sense), switch these to just construct a on the stack. This forces instantiation of the implicit constructor, which will mark the vtable as used.</div><div><br></div><div>* The exception is -fapple-kext mode: In this mode, qualified calls to virtual functions (`a->Base::f()`) still go through the vtable, and since the vtable pointer off this doesn't point to Base's vtable, this needs to reference Base's vtable directly. To keep this working, keep referencing the vtable for virtual calls in apple kext mode. There's 0 test coverage for this :-/</div><span class="HOEnZb"><font color="#888888"><div><br></div><div>Nico</div></font></span></div>
</blockquote></div><br></div>