Differential D99160
[X86][FastISEL] Support DW_TAG_call_site_parameter with FastISEL Authored by alok on Mar 23 2021, 2:21 AM.
Details DW_OP_entry_value and DW_TAG_call_site_parameter work together to preserve optimized out paramters value for debugging purpose. Adding support to it now.
Diff Detail
Event TimelineComment Actions Cool -- thanks for working on this!
Comment Actions Updated to incorporate comments from @djtodoro
and fixed clang-format issue. Comment Actions FastISel is normally used only at -O0, I wouldn't expect any parameters to be optimized out at -O0. If flang is forcing fast-isel even with optimization, that sounds like a problem for flang to solve, not LLVM. Comment Actions Thanks for your comments. I am sorry as the earlier used options in test case were more confusing than helpful. I have changed that now. Comment Actions
I am wondering if this is true... Comment Actions You can try with testcase attached with this patch. bash$ llc -O0 -filetype=obj llvm/test/DebugInfo/X86/callsitepar-fastisel.ll -o callsitepar-fastisel.o [0x00000000000000bf, 0x0000000000000101): DW_OP_GNU_entry_value(DW_OP_reg4 RSI)) Comment Actions I think that the Debug Entry Values feature should not be enabled by default for non optimized code, so the TargetOptions::ShouldEmitDebugEntryValues() should be patched with checking of optimization level (it should be > 0). Comment Actions That's currently intended to be already handled by the frontend, right? (clang only sets EnableDebugEntryValues (which ShouldEmitDebugEntryValues checks (hmm, it checks under 'or', not 'and', so I'm not sure where the "only above -O0" is implemented, but it is implemented somewhere?) if optimizations are enabled, yeah?) Oh, is entry_values actually not conditionalized? It's only the call_site support that's currently conditionalized on "above -O0"? Hmm - If that's the case, and we currently have some cases where entry_values are emitted at -O0, I'm not sure /not/ emitting those is the right call either. If we believe/have data to show that there are so few useful uses of entry_value at -O0 that it's not worth the DWARF size growth to put call_sites in at -O0, then I think it might still be worth leaving the entry_values in (unless they take up a bunch of extra space) for the cases of mixed optimization compilation (-O0 some code you're debugging, but building the rest with optimizations). Comment Actions Looks like there is no explicit check of optimization level (above "-O0"), neither on frontend nor backend for entry-values generation. I think it is the situation since there should not be any optimization (at least that I am aware of, in the case of C/C++) that would cause the entry-values generation...
Yeah... That is valuable example... I am thinking in that direction as well, and I am closer to the enabling it for -O0 case if that is useful (and there is no dramatic cost in terms of DWARF size). Comment Actions Does anyone have this example (where DW_OP_entry_value is used at -O0)? It'd be great to look at it & see if it's a case of unnecessarily losing the location, or legitimately losing it and using entry_value for best-effort recovery (& then a question of whether the loss is appropriate at -O0, or if we want to pessimize -O0 further to avoid the loss). I'd worry about turning on call_sites at -O0 - there'd be a lot more calls (especially for C++ code with lots of implicit operations), but numbers will be needed in any case, so not worth much speculation. Comment Actions Sorry for late response. ````````````````````````````````````` diff --git a/clang/lib/CodeGen/CGDebugInfo.cpp b/clang/lib/CodeGen/CGDebugInfo.cpp
+++ b/clang/lib/CodeGen/CGDebugInfo.cpp llvm::DINode::DIFlags CGDebugInfo::getCallSiteRelatedAttrs() const {
+ Call site-related attributes are useful when there's a possibility of DebugKind == codegenoptions::LocTrackingOnly) return llvm::DINode::FlagZero; ````````````````````````````````````` With this patch Clang/Clang++ turn on LLVM IR flag "DIFlagAllCallsDescribed", in absence of this LLVM does not generate call-site. With the above patch applied below is the comparison of sizes of shared libraries. Name of shared library - Size (without callsite) - Size (with callsites) - % incresase in size ``````````````````````````````````````````````````````````````````````````````````````````````````````````````````````````````````````````` PipSqueak.so 73192 75048 2% ````````````````````````````````````````````````````````````````````` sum 79615057 83995857 5% So the conclusion is,
Comment Actions I think this ^ still needs understanding/investigation. Do you have an example with OP_entry_value at -O0?
It doesn't seem to me, so far, like this is a place where Flang and Clang should diverge - they're both doing the same sort of thing for the same reasons/likely with the same sort of tradeoffs of location accuracy V size cost.
I'd actually be somewhat more worried about object size, rather than/in addition to executable size, due to the increase in relocations (especially with DWARFv5 (& especially with the -mllvm -minimize-addr-in-v5=Ranges which further reduces debug_addr, but can't handle labels and call sites (unlike -minimize-addr-in-v5=Expressions or -minimize-addr-in-v5=Form)), which does a lot to reduce the number of relocations by using debug_addr and address sharing on debug_rnglists/loclists/etc) which can't be compressed, etc. Comment Actions Below is the comparison of total size of object files (*.o) for same build (default (O0) + -g). sum 3535044960 3680496672 4% I shall come back with numbers for options O0 -gdwarf-5 -mllvm -minimize-addr-in-v5=Ranges . Comment Actions Below are the results. PipSqueak.so 74376 75048 0% Comment Actions ~5% object size growth (probably more heavily weighted towards object size when using Split DWARF (most of the growth is likely in the .debug_addr/rela.debug_addr in the .o file, rather than in the DIEs in the .dwo file)) seems like enough that I wouldn't really be super happy about this. Can we come back to this question that seems like it still hasn't been addressed: Does anyone have this example (where DW_OP_entry_value is used at -O0)? It'd be great to look at it & see if it's a case of unnecessarily losing the location, or legitimately losing it and using entry_value for best-effort recovery (& then a question of whether the loss is appropriate at -O0, or if we want to pessimize -O0 further to avoid the loss). (it seems like there's no flang-specific argument here that I've seen - if we are creating DW_OP_entry_value at -O0 then that's a debuggability problem for both Clang and Flang and we should be adopting the same solution here, so unless there's something Flang specific, the "this is harmless to clang and good for flang, so why don't we add the flag" doesn't seem to hold/be relevant here) | ||||||||||||
I think this should be a different patch.
Are you saying this is useful for some -O0 cases? Can you please provide a test case? Is this related to Fortran only?