[PATCH] D15977: [Clang] Supporting all entities declared in lexical scope in LLVM debug info

[Amjad Aboud via cfe-commits]

aaboud marked an inline comment as done.
aaboud added a comment.

Thanks David for the comments.
See answer below.


================
Comment at: lib/CodeGen/CGDebugInfo.cpp:1489
@@ +1488,3 @@
+  if (I != LexicalBlockMap.end()) {
+    RetainedTypes.push_back(Ty.getAsOpaquePtr());
+    return I->second;
----------------
dblaikie wrote:
> aaboud wrote:
> > dblaikie wrote:
> > > Why does the type need to be added to the retained types list?
> > Excellent question :)
> > This was your suggestion few months ago.
> > The reason is that backend will need to know what types are declared in lexical blocks.
> > Imported entities are collected in the import entity list, while types are collected in the retained types.
> > 
> > Check line 518 at DwarfDebug.cpp (http://reviews.llvm.org/D15976#51cfb106)
> Not quite following - could we discover the types lazily as we're generating DWARF in the backend - if we happen to emit a variable of that type & generate the DIE* for the type, etc? (so that if a type becomes unused it's able to be dropped - rather than being preserved only when the type is local)
> 
> I suppose that would make it difficult to choose which scopes we had to preserve when emitting debug info in the backend (if we didn't see a type until after the scope was created - we wouldn't've known whether to emit/create that scope or collapse it into its parent, etc) - so this probably isn't possible/practical.
Your analysis is correct.
I just tried to re-check how we can discover types lazily, but I could not figure out how to make sure the lexical scope would be created for these types.

I guess we can revisit this issue later and re-think a better implementation that make sure optimized types will not be emitted in the debug info. However, I think we can do that in a separate (following) patch.

================
Comment at: test/CodeGenCXX/debug-info-lb.cpp:14
@@ +13,3 @@
+    static int bar = 0;
+    {
+      X a(x);
----------------
dblaikie wrote:
> This block seems unnecessary, and its contents are probably more complicated than needed. This seems to suffice:
> 
>   X a;
>   Y b;
> 
> & the global variable is emitted without referencing it anyway.
> 
> (you could drop the int return and x parameter, too - no need to return anything from the function, and the if (x) could be removed - the scope can remain & still produce the same debug info I think)
OK, I simplify the test, and it is still does what I needed.

The only thing that I need to clarify is the extra block scope that I added to wrap the "a" and "b" variable definition.
The reason I did that is to test the case where the types "X" and "Y" are defined in a different scope than they are used at.
Without handling this case correctly, the LLVM IR could mistakenly associate "X" and "Y" to the scope of the usage and not the definition, i.e. the scope of "a" and "b", to test that I had to have different scope as in the example.
I added a comment in the test explaining that.


http://reviews.llvm.org/D15977