[PATCH] D93441: [DebugInfo] Fix crash with -fdebug-info-for-profiling and split dwarf

[David Blaikie via Phabricator via llvm-commits]

dblaikie added a comment.

Any idea if this was a recent regression or the like? Worth noting what patch might've caused it, etc.



================
Comment at: llvm/lib/CodeGen/AsmPrinter/DwarfCompileUnit.cpp:985-987
+  // -fdebug-info-for-profiling requires a subprogram DIE.
+  if (includeMinimalInlineScopes() && !getCUNode()->getDebugInfoForProfiling())
     ContextDIE = &getUnitDie();
----------------
Idle thoughts, not especially related to this patch:

This may highlight an interesting situation:

Currently gmlt + split-dwarf-inlining + fission => non-fission gmlt, because split-dwarf-inlining is essentially the gmlt-like data, kept in the object file. So if the only thing that's being fissioned into the dwo is gmlt data anyway, and the gmlt-like data is also going in the object file, why bother with the dwo gmlt data? But if debug-info-for-profiling makes for particularly larger debug info, then it could be reasonable to have gmlt + split-dwarf-inlining + fission + debug-info-for-profiling all active at once. (though, perhaps now we have a good way to turn fission off (-gno-split-dwarf) perhaps we could remove that quirk I added that caused gmlt + split-dwarf-inlining + fission not to compos, but overriding/disabling fission) 

Might be interesting to measure object/executable size metrics for debug-info-for-profiling + gmlt and debug-info-for-profiling + gmlt + fission + split-dwarf-inlining. See how much obj/exe space is saved by that and whether it's worth supporting.


================
Comment at: llvm/test/DebugInfo/X86/debug-info-for-profiling-crash.ll:1
+; RUN: llc %s -split-dwarf-file=%t.dwo -filetype=obj -o /dev/null
+
----------------
Generally tests should verify more than "does anything other than crash" - that's a fairly broad test/definition of acceptable behavior. So some checking that the resulting DWARF is as desired - especially around the specific codepath that crashed (what construct was meant to be created but never got created because of the assertion, for instance - or, with a non-assertions build did it produce some specific mangled DWARF that we can check is now correct)


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D93441/new/

https://reviews.llvm.org/D93441