On Wed, Nov 5, 2014 at 1:47 PM, Frédéric Riss <[email protected]> wrote:
> > On Nov 5, 2014, at 1:33 PM, David Blaikie <[email protected]> wrote: > > > > On Wed, Nov 5, 2014 at 11:19 AM, Frederic Riss <[email protected]> wrote: > >> Author: friss >> Date: Wed Nov 5 13:19:04 2014 >> New Revision: 221385 >> >> URL: http://llvm.org/viewvc/llvm-project?rev=221385&view=rev >> Log: >> [DebugInfo] Do not record artificial global initializer functions in the >> DeclCache. >> >> When we are generating the global initializer functions, we call >> CGDebugInfo::EmitFunctionStart() with a valid decl which is describing >> the initialized global variable. Do not update the DeclCache with this >> key as it will overwrite the the cached variable DIGlobalVariable with >> the newly created artificial DISubprogram. >> >> One could wonder if we should put artificial subprograms in the DIE tree >> at all (there are vaild uses for them carrying line information though). >> >> Modified: >> cfe/trunk/lib/CodeGen/CGDebugInfo.cpp >> cfe/trunk/test/CodeGenCXX/debug-info-namespace.cpp >> >> Modified: cfe/trunk/lib/CodeGen/CGDebugInfo.cpp >> URL: >> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGDebugInfo.cpp?rev=221385&r1=221384&r2=221385&view=diff >> >> ============================================================================== >> --- cfe/trunk/lib/CodeGen/CGDebugInfo.cpp (original) >> +++ cfe/trunk/lib/CodeGen/CGDebugInfo.cpp Wed Nov 5 13:19:04 2014 >> @@ -2527,7 +2527,10 @@ void CGDebugInfo::EmitFunctionStart(Glob >> getOrCreateFunctionType(D, FnType, Unit), Fn->hasInternalLinkage(), >> true /*definition*/, ScopeLine, Flags, CGM.getLangOpts().Optimize, >> Fn, >> TParamsArray, getFunctionDeclaration(D)); >> - if (HasDecl) >> + // We might get here with a VarDecl in the case we're generating >> + // code for the initialization of globals. Do not record these decls >> + // as they will overwrite the actual VarDecl Decl in the cache. >> + if (HasDecl && isa<FunctionDecl>(D)) >> DeclCache.insert(std::make_pair(D->getCanonicalDecl(), >> llvm::WeakVH(SP))); >> >> // Push the function onto the lexical block stack. >> >> Modified: cfe/trunk/test/CodeGenCXX/debug-info-namespace.cpp >> URL: >> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGenCXX/debug-info-namespace.cpp?rev=221385&r1=221384&r2=221385&view=diff >> >> ============================================================================== >> --- cfe/trunk/test/CodeGenCXX/debug-info-namespace.cpp (original) >> +++ cfe/trunk/test/CodeGenCXX/debug-info-namespace.cpp Wed Nov 5 >> 13:19:04 2014 >> @@ -5,8 +5,8 @@ >> namespace A { >> #line 1 "foo.cpp" >> namespace B { >> -int i; >> -void f1() { } >> +extern int i; >> +int f1() { return 0; } >> void f1(int) { } >> struct foo; >> struct bar { }; >> @@ -19,7 +19,7 @@ using namespace B; >> >> using namespace A; >> namespace E = A; >> - >> +int B::i = f1(); >> int func(bool b) { >> if (b) { >> using namespace A::B; >> @@ -51,8 +51,8 @@ using B::i; >> // CHECK: [[FILE]] {{.*}}debug-info-namespace.cpp" >> // CHECK: [[BAR:![0-9]*]] {{.*}} ; [ DW_TAG_structure_type ] [bar] [line >> 6, {{.*}}] [decl] [from ] >> // CHECK: [[F1:![0-9]*]] {{.*}} ; [ DW_TAG_subprogram ] [line 4] [def] >> [f1] >> -// CHECK: [[FUNC:![0-9]*]] {{.*}} ; [ DW_TAG_subprogram ] {{.*}} [def] >> [func] >> // CHECK: [[FILE2]]} ; [ DW_TAG_file_type ] [{{.*}}foo.cpp] >> +// CHECK: [[FUNC:![0-9]*]] {{.*}} ; [ DW_TAG_subprogram ] {{.*}} [def] >> [func] >> > > Is there a more specific observable change we should be testing here? I > assume if this produced a bug, it would be actually visible in some way not > just "metadat order changed slightly". > > > You misunderstood the change in the testcase. The metadata order change is > a side effect from the patch, but it’s not this part that tests the change. > (This kinda random order is a pain BTW. While I was investigating various > fixes for that issue, I had to dig into quite a few failing test cases just > to find out that things had only been reordered) > > I changed the global variable B::i to need an initializer function. If you > run the version of the testcase I committed through a clang that hasn’t the > patch, you’ll see that the import directive will point to the a global init > function rather than to the variable. The real test performed wrt the patch > is that the import directive really points to the variable and this test > was already there and thus doesn’t appear in the patch. > Right right - thanks! > > Fred > > // CHECK: [[I:![0-9]*]] = metadata !{metadata !"0x34\00i\00{{.*}}", >> metadata [[NS]], {{.*}} ; [ DW_TAG_variable ] [i] >> // CHECK: [[MODULES]] = metadata !{metadata [[M1:![0-9]*]], metadata >> [[M2:![0-9]*]], metadata [[M3:![0-9]*]], metadata [[M4:![0-9]*]], metadata >> [[M5:![0-9]*]], metadata [[M6:![0-9]*]], metadata [[M7:![0-9]*]], metadata >> [[M8:![0-9]*]], metadata [[M9:![0-9]*]], metadata [[M10:![0-9]*]], metadata >> [[M11:![0-9]*]], metadata [[M12:![0-9]*]], metadata [[M13:![0-9]*]]} >> // CHECK: [[M1]] = metadata !{metadata !"0x3a\0011\00", metadata >> [[CTXT]], metadata [[NS]]} ; [ DW_TAG_imported_module ] >> >> >> _______________________________________________ >> cfe-commits mailing list >> [email protected] >> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits > > >
_______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
