[patch] Don't let virtual calls and dynamic casts call MarkVTableUsed()

Nico Weber thakis at chromium.org
Tue Jan 13 11:22:57 PST 2015

On Tue, Jan 13, 2015 at 11:11 AM, Reid Kleckner <rnk at google.com> wrote:

> 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.

At the moment, the only thing in coding adding to the DeferredVTables
vector in codegen is getAddrOfVTable(), and that's only called for structor
body emission (and apple kext vcalls). So I think this should be fine.

> 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:
> struct A {
>   virtual void f();
> };
> static void h(A &a) {
>   a.f(); // requires inlining, must devirtualize in LLVM, requires
> available_externally vtable
> }
> void g() {
>   A a;
>   a.f(); // obvious, devirtualized in frontend
>   h(a);
> }

This is discussed in http://llvm.org/bugs/show_bug.cgi?id=20337#c18 (sorry,
got the comment number wrong above). In this case, we're still able to
optimize (in theory) since A has an implicit constructor that gets emitted
which does cause emission of a vtable. If you make the constructor explicit
and out of line (as in that bug comment), that's no longer true – but if we
end up implementing that at some point, we can worry about it then.

> On Tue, Jan 13, 2015 at 10:40 AM, Nico Weber <thakis at chromium.org> wrote:
>> Hi,
>> 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.
>> 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).
>> 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.
>> 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:
>>     template <typename T> struct OwnPtr {
>>       ~OwnPtr() { static_assert((sizeof(T) > 0), "TypeMustBeComplete"); }
>>     };
>>     class ScriptLoader;
>>     struct Base { virtual ~Base(); };
>>     struct Sub : public Base {
>>       virtual void someFun() const {}
>>       OwnPtr<ScriptLoader> m_loader;
>>     };
>>     void f(Sub *s) { s->someFun(); }
>> 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.
>> This is similar to r213109 in spirit. r225761 was a prerequisite for this
>> change.
>> 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.
>> * 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 :-/
>> Nico
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20150113/647e1686/attachment.html>

More information about the cfe-commits mailing list