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

Hans Wennborg hans at chromium.org
Thu Jun 5 22:11:11 PDT 2014


================
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?

http://reviews.llvm.org/D4038






More information about the cfe-commits mailing list