On Mon, May 10 2021, Richard Biener wrote: > On Tue, Apr 27, 2021 at 5:26 PM Martin Jambor <mjam...@suse.cz> wrote: >> >> Hi, >> >> Whereas the previous patch fixed issues with code left behind after >> IPA-SRA removed a parameter but only reset all affected debug bind >> statements, this one updates them with expressions which can allow the >> debugger to print the removed value - see the added test-case. >> >> Even though I originally did not want to create DEBUG_EXPR_DECLs for >> intermediate values, I ended up doing so, because otherwise the code >> started creating statements like >> >> # DEBUG __aD.198693 => &MEM[(const struct _Alloc_nodeD.171110 >> *)D#195]._M_tD.184726->_M_implD.171154 >> >> which not only is a bit scary but also gimple-fold ICEs on >> it. Therefore I decided they are probably quite necessary and have >> them. >> >> The patch simply notes each removed SSA name present in a debug >> statement and then works from it backwards, looking if it can >> reconstruct the expression it represents (which can fail if a >> non-degenerate PHI node is in the way). If it can, it populates two >> hash maps with those expressions so that 1) removed assignments are >> replaced with a debug bind defining a new intermediate debug_decl_expr >> and 2) existing debug binds that refer to SSA names that are bing >> removed now refer to corresponding debug_decl_exprs. > > Isn't this what insert_debug_temp_for_var_def already does when you > remove a stmt and if you take care to do that back-to-front? So with > IPA SRA removing a parameter you'd "only" need to make sure to > set up a debug stmt for the parameter itself and that be picked up > for the (uninitialized) default-def you map to? >
But there is no removal, the dead statements creating dead SSAs are not even copied when tree-inline.c does its thing, such SSAs are actually mapped to error_mark_node. The code is heavily inspired by what removal does but (IIRC I hope) it is also much simpler because IPA-SRA can only remove limited classes of scalars. Martin >> If a removed parameter is passed to another function, the debugging >> information still cannot describe its value there - see the xfailed >> test in the testcase. I sort of know what needs to be done but the >> handling of debug information for removed parameters is LTO unfriendly >> in general and so needs a bit more work. >> >> Bootstrapped and tested on x86_64-linux, i686-linux and aarch64-linux. >> Also LTO-bootstrapped and LTO-profiledbootstrapped on x86_64-linux. >> >> OK for trunk? >> >> Thanks, >> >> Martin >> >> >> gcc/ChangeLog: >> >> 2021-03-29 Martin Jambor <mjam...@suse.cz> >> >> PR ipa/93385 >> * ipa-param-manipulation.h (class ipa_param_body_adjustments): New >> members remap_with_debug_expressions, m_dead_ssa_debug_equiv, >> m_dead_stmt_debug_equiv and prepare_debug_expressions. Added >> parameter to mark_dead_statements. >> * ipa-param-manipulation.c: Include tree-phinodes.h and cfgexpand.h. >> (ipa_param_body_adjustments::mark_dead_statements): New parameter >> debugstack, push into it all SSA names used in debug statements, >> produce m_dead_ssa_debug_equiv mapping for the removed param. >> (replace_with_mapped_expr): New function. >> (ipa_param_body_adjustments::remap_with_debug_expressions): Likewise. >> (ipa_param_body_adjustments::prepare_debug_expressions): Likewise. >> (ipa_param_body_adjustments::common_initialization): Gather and >> procecc SSA which will be removed but are in debug statements. >> Simplify. >> (ipa_param_body_adjustments::ipa_param_body_adjustments): Initialize >> new members. >> * tree-inline.c (remap_gimple_stmt): Create a debug bind when >> possible >> when avoiding a copy of an unnecessary statement. Remap removed SSA >> names in existing debug statements. >> (tree_function_versioning): Do not create DEBUG_EXPR_DECL for removed >> parameters if we have already done so. >> >> gcc/testsuite/ChangeLog: >> >> 2021-03-29 Martin Jambor <mjam...@suse.cz> >> >> PR ipa/93385 >> * gcc.dg/guality/ipa-sra-1.c: New test.
