[PATCH] [ms-cxxabi] Emit linkonce complete dtors in TUs that need them

[Reid Kleckner]

On Fri, Jul 19, 2013 at 1:46 PM, John McCall <rjmccall at apple.com> wrote:

> On Jul 15, 2013, at 10:03 AM, Reid Kleckner <rnk at google.com> wrote:
> > http://llvm-reviews.chandlerc.com/D1066
>
> Okay, so what I'm understanding here is that, when MSVC emits a destructor
> (for example, when it's out of line), it emits a global symbol which
> evaluates
> the destructor body and also destroys the non-virtual-base subobjects,
> which
> is basically the behavior of Itanium base destructors.  If it needs a
> function
> that destroys the entire thing (the behavior of the Itanium complete
> destructor),
> it emits it ad hoc, using a symbol that'll be uniqued within the module.
>

Yep, that's all correct.  This is the mapping from Itanium terminology to
the names that native Windows tools emit:

Base -> this is just called ~Class
Complete -> vbase dtor (calls the base dtor and all vbase dtors)
Deleting -> scalar or vector deleting dtor

This is documented in the mangler, which isn't the most obvious place to
look I suppose.

Meanwhile, the vf-table for a virtual destructor has entrypoints for what,
> exactly?  There seems to be a vector deleting destructor, presumably used
> for delete[] (which doesn't actually need to be virtually dispatched, but
> that
> decision may post-date MSVC's decision).  There's also a scalar deleting
> destructor, used for delete?  How do they generate direct calls to the
> destructor,
> i.e. foo->~Foo()?


Timur answered that.

Anyway, this should all be in a nice, detailed comment somewhere in
> MSCXXABI.cpp.
>

Done.

-bool CodeGenModule::MayDeferGeneration(const ValueDecl *Global) {
> +bool CodeGenModule::MayDeferGeneration(GlobalDecl GD) {
>
> Instead of doing this, you should just make non-deferred generation only
> emit
> the variants that it actually guarantees, which seems to be just the base
> variant.
>

OK.


> +  // XXX: In the Microsoft ABI, we want to emit a delegating complete dtor
> +  // without a definition, which means we won't be able to tell if the
> +  // definition is a try body.  In this case, MSVC simply delegates, so
> we do
> +  // the same.
>
> What is this XXX about?  Is this a FIXME?  It looks like we're actually
> doing it.
>

I should remove that.  I'm still confused about why destructors defined as
try bodies need to be a special case, but the current behavior should be
compatible with cl.exe.


> +  // If the class has no virtual bases, then the complete and base
> destructors
> +  // are equivalent, for all C++ ABIs supported by clang.  We can save on
> code
> +  // size by calling the base dtor directly, especially if we'd have to
> emit a
> +  // thunk otherwise.
> +  // FIXME: We could do this for Itanium, but we should consult with John
> first.
>
> Okay, you can't put this in a comment. :)
>
> I'm fine with unconditionally emitting calls to base dtors in this case.
>

It should even be a win on Darwin, since -mconstructor-aliases is disabled
there.  I'll make that a separate change in case it breaks something.

+  if (getCXXABI().useThunkForDtorVariant(dtorType) &&
> +      dtorType == Dtor_Complete && dtor->getParent()->getNumVBases() == 0)
>
> Checking dtorType and getNumVBases() are basically free because they
> don't require a real function call.  You should do those first.  Or
> actually, I
> guess you don't need the check.
>

Reordered.  I'm leaving it for now, but planning to remove it.


> Otherwise this looks good.
>
> John.
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20130719/d380964a/attachment.html>