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

[Richard Smith]

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 =)
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20140606/97447b30/attachment.html>