llvm dev - Nov 2013 - [LLVMdev] DebugInfo: LTO Metadata Size reduction by removing some cycles

>From a thread with Adrian on llvm-commits I looked a little at cases where
DwarfCompileUnit's getOrCreateContextDIE's "fallback to return the
CU"
didn't fire when a CU DIE was required (ie: when the "getDIE" call
actually
found the CU before the fallback happened)

This seems unnecessary, and any case where we do this (where a metadata
node has a non-null context DIE pointing to the CU) is just wasted space.
By ensuring we always use null to refer to the CU we'll reduce cycles and
metadata nodes in LTO situations allowing the nodes to be re-used across
CUs in LTO cases.

I've attached the CLang patch to do this, but I don't want to commit
this
until the LLVM test cases have been cleaned up (the easiest way to find
these test cases would be to add an assert in
CompileUnit::getORCreateContextDIE. Changing this:

  if (DIE *ContextDIE = getDIE(Context))
    return ContextDIE;

to this:

  if (DIE *ContextDIE = getDIE(Context)) {
    assert(ContextDIE != CUDie);
    return ContextDIE;
  }

Then finding all the test cases that fail and null out any context fields
that refer to the CU, thus relying on the fallback instead.

Since we're not working on LTO right now, cleaning up those test cases
(20-30, it's not a lot but more than I have time for) isn't something I
can
do right now but seemed like it might be interesting to you. And I figured
you'd be able to see if this helps size much, whereas I've not been
looking
at/analyzing LTO size costs.
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20131115/d922ab4c/attachment.html>
-------------- next part --------------
diff --git lib/CodeGen/CGDebugInfo.cpp lib/CodeGen/CGDebugInfo.cpp
index 9b131fd..c75fd05 100644
--- lib/CodeGen/CGDebugInfo.cpp
+++ lib/CodeGen/CGDebugInfo.cpp
@@ -138,7 +138,7 @@ void CGDebugInfo::setLocation(SourceLocation Loc) {
 /// getContextDescriptor - Get context info for the decl.
 llvm::DIScope CGDebugInfo::getContextDescriptor(const Decl *Context) {
   if (!Context)
-    return TheCU;
+    return llvm::DIScope();
 
   llvm::DenseMap<const Decl *, llvm::WeakVH>::iterator
     I = RegionMap.find(Context);
@@ -155,7 +155,7 @@ llvm::DIScope CGDebugInfo::getContextDescriptor(const Decl
*Context) {
     if (!RDecl->isDependentType())
       return getOrCreateType(CGM.getContext().getTypeDeclType(RDecl),
                                         getOrCreateMainFile());
-  return TheCU;
+  return llvm::DIScope();
 }
 
 /// getFunctionName - Get function name for the given FunctionDecl. If the
diff --git test/CodeGenCXX/debug-info-namespace.cpp
test/CodeGenCXX/debug-info-namespace.cpp
index a2d7ede..03e70c7 100644
--- test/CodeGenCXX/debug-info-namespace.cpp
+++ test/CodeGenCXX/debug-info-namespace.cpp
@@ -52,8 +52,8 @@ int func(bool b) {
 // CHECK: [[I:![0-9]*]] = {{.*}}, metadata [[NS]], metadata !"i",
{{.*}} ; [ 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]*]]}
 // CHECK: [[M1]] = metadata !{i32 {{[0-9]*}}, metadata [[CTXT]], metadata
[[NS]], i32 11} ; [ DW_TAG_imported_module ]
-// CHECK: [[M2]] = metadata !{i32 {{[0-9]*}}, metadata [[CU]], metadata
[[CTXT]], i32 {{[0-9]*}}} ; [ DW_TAG_imported_module ]
-// CHECK: [[M3]] = metadata !{i32 {{[0-9]*}}, metadata [[CU]], metadata
[[CTXT]], i32 15, metadata !"E"} ; [ DW_TAG_imported_module ]
+// CHECK: [[M2]] = metadata !{i32 {{[0-9]*}}, null, metadata [[CTXT]], i32
{{[0-9]*}}} ; [ DW_TAG_imported_module ]
+// CHECK: [[M3]] = metadata !{i32 {{[0-9]*}}, null, metadata [[CTXT]], i32 15,
metadata !"E"} ; [ DW_TAG_imported_module ]
 // CHECK: [[M4]] = metadata !{i32 {{[0-9]*}}, metadata [[LEX2:![0-9]*]],
metadata [[NS]], i32 19} ; [ DW_TAG_imported_module ]
 // CHECK: [[LEX2]] = metadata !{i32 {{[0-9]*}}, metadata [[FILE2]], metadata
[[LEX1:![0-9]+]], i32 {{[0-9]*}}, i32 0, i32 {{.*}}} ; [ DW_TAG_lexical_block ]
 // CHECK: [[LEX1]] = metadata !{i32 {{[0-9]*}}, metadata [[FILE2]], metadata
[[FUNC]], i32 {{[0-9]*}}, i32 0, i32 {{.*}}} ; [ DW_TAG_lexical_block ]

Hi David,

Thanks for the patch. Yes, I am interested in removing cycles in debug info
MDNodes. One of the reasons that we create new copies of MDNodes in the
linked module is cycles.
But I may not get to this soon.

Manman


On Fri, Nov 15, 2013 at 1:41 PM, David Blaikie <dblaikie at gmail.com>
wrote:
> From a thread with Adrian on llvm-commits I looked a little at cases where
> DwarfCompileUnit's getOrCreateContextDIE's "fallback to return
the CU"
> didn't fire when a CU DIE was required (ie: when the "getDIE"
call actually
> found the CU before the fallback happened)
>
> This seems unnecessary, and any case where we do this (where a metadata
> node has a non-null context DIE pointing to the CU) is just wasted space.
> By ensuring we always use null to refer to the CU we'll reduce cycles
and
> metadata nodes in LTO situations allowing the nodes to be re-used across
> CUs in LTO cases.
>
> I've attached the CLang patch to do this, but I don't want to
commit this
> until the LLVM test cases have been cleaned up (the easiest way to find
> these test cases would be to add an assert in
> CompileUnit::getORCreateContextDIE. Changing this:
>
>   if (DIE *ContextDIE = getDIE(Context))
>     return ContextDIE;
>
> to this:
>
>   if (DIE *ContextDIE = getDIE(Context)) {
>     assert(ContextDIE != CUDie);
>     return ContextDIE;
>   }
>
> Then finding all the test cases that fail and null out any context fields
> that refer to the CU, thus relying on the fallback instead.
>
> Since we're not working on LTO right now, cleaning up those test cases
> (20-30, it's not a lot but more than I have time for) isn't
something I can
> do right now but seemed like it might be interesting to you. And I figured
> you'd be able to see if this helps size much, whereas I've not been
looking
> at/analyzing LTO size costs.
>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20131118/d7a623bb/attachment.html>