[PATCH] D15976: Supporting all entities declared in lexical scope in LLVM debug info
Amjad Aboud via llvm-commits
llvm-commits at lists.llvm.org
Mon Feb 15 05:53:33 PST 2016
aaboud added a comment.
Sorry for the late response, please see new patch and below comments.
================
Comment at: lib/CodeGen/AsmPrinter/DwarfCompileUnit.cpp:20
@@ -18,1 +19,3 @@
+static cl::opt<bool> AbstractOriginOnLS(
+ "use-abstract-origin-on-ls", cl::Hidden,
----------------
dblaikie wrote:
> I'd really prefer not to have this option/backcompat feature.
>
> Do you have a need to support older GDBs (or LLDB) with this change? Otherwise I think this is a sufficiently narrow case we could probably take the change optimistically and let other debuggers deal with it as a bug. I don't think it'll disrupt that many users. (though I could be wrong/happy to deal with it differently if we have contributing developers who feel strongly that they need a workaround for their users until their platform debuggers catch up)
>
> I realize it doesn't cost too much (your implementation has made it quite simple to support both) - this isn't all teh cost. We'd have to plumb the option up through into the frontend, or make it a debugger tuning feature (how would we tune it for GDB if we want to support old GDBs here? I would imagine this should be a "do the good thing by default, but fall back to the bad thing if we're specifically targeting known bad compilers, like LLDB" - though I think Adrian mentioned he'd be willing to just take this as an LLDB bug, but I'm not 100% sure)
OK, I agree.
I just wanted to show you (record) the changes need to be done to support older debuggers.
I will stick to the new behavior, and assume LLDB will have a fix soon to suit this new behavior.
If we get any complains from Clang users we can reconsider support the workaround.
Do you agree to this?
================
Comment at: lib/CodeGen/AsmPrinter/DwarfDebug.cpp:547
@@ +546,3 @@
+ for (const auto &I : CUMap)
+ forBothCUs(*I.second, [&](DwarfCompileUnit &CU) {
+ CU.finishLocalScopeDefinitions();
----------------
OK, I updated the test to run also with "split-dwarf=ENABLE", and even check manually against older behavior, and in both cases the debug info is emitted to the ".debug_dwo" section.
So, I guess the current implementation is fine, what do you think?
http://reviews.llvm.org/D15976
More information about the llvm-commits
mailing list