> 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]
> <mailto:[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
> <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
>
> <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
>
> <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.
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] <mailto:[email protected]>
> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
> <http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits>
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
