[PATCH] D29310: Fix DwarfDebug assertions with LTO mixing -g and -gmlt

David Blaikie via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Tue Jan 31 08:38:28 PST 2017


dblaikie added a comment.

Given the complexity of the test cases, this might be easier as two separate reviews.

The test cases should be simplified a fair bit further, I think - especially removing user defined types should be helpful in reducing the amount of debug info metadata and possibly making things clearer. (also, in the second test it mentions the possibility of removing the (non-inlined) definition of 'foo' - mark 'foo' as inline in the source and this should happen automatically. If possible, use __attribute__((always_inline)) to ensure the test case is fairly optimization independent, but I guess since it needs to optimize away the parameter alloca it won't be doable at -O0 anyway, so maybe that's not important)



================
Comment at: lib/CodeGen/AsmPrinter/DwarfDebug.cpp:1151
+  // verify this.
+  PrevAVSize = AbstractVariables.size();
+
----------------
Rather than adding a new member variable & diagnosing this case where we were before - perhaps it'd make for a more actionable assertion if we change how we're doing this.

There's exactly one place that's (admittedly, currently - this could change, so might make what I'm proposing more brittle) creating AbstractVariables - in DwarfDebug::createAbstractVariable.

>From that function it should be possible to check the debug info level (CurFn->getFunction()->getSubprogram()->getUnit()->getEmissionKind()) and assert that it is not LineTablesOnly. That way it'll fail faster/more informatively, and requires less wide-scoped state tracking, etc.


================
Comment at: lib/CodeGen/AsmPrinter/DwarfDebug.cpp:1225-1229
+    // With LTO, a function compiled -gmlt could inline a function compiled -g,
+    // which could have all real instructions optimized/folded away, which in
+    // turn means no abstract scopes would be created. But the DbgValues will
+    // still be there.  Forget them.
+    DbgValues.clear();
----------------
Do bad things happen if we don't clear this? Might be useful if the comment described what undesirable work this avoids?


https://reviews.llvm.org/D29310





More information about the llvm-commits mailing list