[PATCH] Defer codegen of dllexport/attribute(used) inline method definitions

[Richard Smith]

On Fri, Jun 6, 2014 at 9:53 AM, Richard Smith <richard at metafoo.co.uk> wrote:

> On Thu, Jun 5, 2014 at 10:11 PM, Hans Wennborg <hans at chromium.org> wrote:
>
>> ================
>> Comment at: lib/CodeGen/ModuleBuilder.cpp:92
>> @@ -91,3 @@
>> -      if (!D->isDependentContext() &&
>> -          (D->hasAttr<UsedAttr>() || D->hasAttr<ConstructorAttr>() ||
>> -           D->hasAttr<DLLExportAttr>())) {
>> ----------------
>> Richard Smith wrote:
>> > Hans Wennborg wrote:
>> > > Richard Smith wrote:
>> > > > An alternative fix would be to check `D->isUsed()` instead of
>> checking for `UsedAttr` here. That'd avoid the locality degradation and
>> memory allocation here.
>> > > >
>> > > > Or just remove this check and unconditionally call
>> `EmitTopLevelDecl`? That would better match what we do for out-of-line
>> method definitions, and would avoid duplicating this between here and
>> `ASTContext::DeclMustBeEmitted`.
>> > > Checking D->isUsed() certainly sounds simpler :-) I'll try that out.
>> > >
>> > > We can't unconditionally call EmitTopLevelDecl, because it will call
>> DeclMustBeEmitted, which might calculate the gva linkage, and then we fail
>> this test from SemaCXX/undefined-internal.cpp :(
>> > >
>> > >       namespace test7 {
>> > >         typedef struct {
>> > >           void bar();
>> > >           void foo() { bar(); }
>> > >         } A;
>> > >       }
>> > >
>> > > I suspect that test doesn't work well with the "used" attribute
>> either.
>> > Arrgh! That's horrible, but it does justify delaying the emission to
>> end of TU. The existing code is wrong for this case even without checking
>> 'used'. A comment here mentioning this case would be useful for future
>> archaeologists.
>> The current patch delays emission to the end of the current top level
>> declaration. Is that OK, or are you saying we need to delay all the way to
>> the end of the TU?
>>
>
> End-of-top-level-decl sounds strictly better than end-of-TU =)
>

... except that the end-of-top-level-decl callback presumably doesn't
happen late enough (if it did, the recursive-class-walk we used to do would
have worked too). And in particular, here:

struct S {
  struct T {
    void foo() {}
    void bar() { foo(); }
  };
};

... I think we'll have no top-level decl emitted after the method bodies
are passed to the consumer.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20140606/89716ce3/attachment.html>