Message ID | ri6fss1zbcz.fsf@suse.cz |
---|---|
State | New |
Headers | show |
Series | Replace more DEBUG_EXPR_DECL creations with build_debug_expr_decl | expand |
On November 12, 2021 3:39:56 PM GMT+01:00, Martin Jambor <mjambor@suse.cz> wrote: >Hi, > >On Tue, Nov 09 2021, Richard Biener wrote: >> On Mon, 8 Nov 2021, Martin Jambor wrote: >>> this patch introduces a helper function build_debug_expr_decl to build >>> DEBUG_EXPR_DECL tree nodes in the most common way and replaces with a >>> call of this function all code pieces which build such a DECL itself >>> and sets its mode to the TYPE_MODE of its type. >>> >>> There still remain 11 instances of open-coded creation of a >>> DEBUG_EXPR_DECL which set the mode of the DECL to something else. It >>> would probably be a good idea to figure out that has any effect and if >>> not, convert them to calls of build_debug_expr_decl too. But this >>> patch deliberately does not introduce any functional changes. >>> >>> Bootstrapped and tested on x86_64-linux, OK for trunk? >> >> OK (the const_tree suggestion is a good one). >> >> For the remaining cases I'd simply use >> >> decl = build_debug_expr_decl (type); >> SET_DECL_MODE (decl) = ...; >> >> and thus override the mode afterwards, maybe adding a comment to >> check whether that's necessary. As said, the only case where it >> might matter is when we create a debug decl replacement for a FIELD_DECL, >> so maybe for those SRA things we create for DWARF "piece" info? >> > >Like this? This patch replaces all but one remaining open coded >constructions of DEBUG_EXPR_DECL with calls to build_debug_expr_decl, >even if - in order not to introduce any functional change - the mode of >the constructed decl is then overwritten. > >It is not clear if changing the mode has any effect in practice and >therefore I have added a FIXME note to code which does it, as >requested. > >After this patch, DEBUG_EXPR_DECLs are created only by >build_debug_expr_decl and make_debug_expr_from_rtl which looks like >it should be left alone. > >Bootstrapped and tested on x86_64-linux. OK for trunk? Yes. Thanks, Richard. >I have also compared the generated DWARF (with readelf -w) of cc1plus >generated by a compiler with this patch and one with the mode setting >removed (on top of this patch) and there were no differences >whatsoever. So perhaps we can just remove it? I have not >bootstrapped that patch yet, though. I guess that for one case it mattered and we might have a testcase to show that and other cases were just cut and pasted from the "wrong" place... Richard. >Thanks, > >Martin > > >gcc/ChangeLog: > >2021-11-11 Martin Jambor <mjambor@suse.cz> > > * cfgexpand.c (expand_gimple_basic_block): Use build_debug_expr_decl, > add a fixme note about the mode assignment perhaps being unnecessary. > * ipa-param-manipulation.c (ipa_param_adjustments::modify_call): > Likewise. > (ipa_param_body_adjustments::mark_dead_statements): Likewise. > (ipa_param_body_adjustments::reset_debug_stmts): Likewise. > * tree-inline.c (remap_ssa_name): Likewise. > (tree_function_versioning): Likewise. > * tree-into-ssa.c (rewrite_debug_stmt_uses): Likewise. > * tree-ssa-loop-ivopts.c (remove_unused_ivs): Likewise. > * tree-ssa.c (insert_debug_temp_for_var_def): Likewise. >--- > gcc/cfgexpand.c | 5 ++--- > gcc/ipa-param-manipulation.c | 17 +++++++---------- > gcc/tree-inline.c | 17 +++++++---------- > gcc/tree-into-ssa.c | 7 +++---- > gcc/tree-ssa-loop-ivopts.c | 5 ++--- > gcc/tree-ssa.c | 5 ++--- > 6 files changed, 23 insertions(+), 33 deletions(-) > >diff --git a/gcc/cfgexpand.c b/gcc/cfgexpand.c >index 55ff75bd78e..eb6466f4be6 100644 >--- a/gcc/cfgexpand.c >+++ b/gcc/cfgexpand.c >@@ -5898,18 +5898,17 @@ expand_gimple_basic_block (basic_block bb, bool disable_tail_calls) > temporary. */ > gimple *debugstmt; > tree value = gimple_assign_rhs_to_tree (def); >- tree vexpr = make_node (DEBUG_EXPR_DECL); >+ tree vexpr = build_debug_expr_decl (TREE_TYPE (value)); > rtx val; > machine_mode mode; > > set_curr_insn_location (gimple_location (def)); > >- DECL_ARTIFICIAL (vexpr) = 1; >- TREE_TYPE (vexpr) = TREE_TYPE (value); > if (DECL_P (value)) > mode = DECL_MODE (value); > else > mode = TYPE_MODE (TREE_TYPE (value)); >+ /* FIXME: Is setting the mode really necessary? */ > SET_DECL_MODE (vexpr, mode); > > val = gen_rtx_VAR_LOCATION >diff --git a/gcc/ipa-param-manipulation.c b/gcc/ipa-param-manipulation.c >index ae3149718ca..a230735d71e 100644 >--- a/gcc/ipa-param-manipulation.c >+++ b/gcc/ipa-param-manipulation.c >@@ -831,9 +831,8 @@ ipa_param_adjustments::modify_call (cgraph_edge *cs, > } > if (ddecl == NULL) > { >- ddecl = make_node (DEBUG_EXPR_DECL); >- DECL_ARTIFICIAL (ddecl) = 1; >- TREE_TYPE (ddecl) = TREE_TYPE (origin); >+ ddecl = build_debug_expr_decl (TREE_TYPE (origin)); >+ /* FIXME: Is setting the mode really necessary? */ > SET_DECL_MODE (ddecl, DECL_MODE (origin)); > > vec_safe_push (*debug_args, origin); >@@ -1063,9 +1062,8 @@ ipa_param_body_adjustments::mark_dead_statements (tree dead_param, > return; > } > >- tree dp_ddecl = make_node (DEBUG_EXPR_DECL); >- DECL_ARTIFICIAL (dp_ddecl) = 1; >- TREE_TYPE (dp_ddecl) = TREE_TYPE (dead_param); >+ tree dp_ddecl = build_debug_expr_decl (TREE_TYPE (dead_param)); >+ /* FIXME: Is setting the mode really necessary? */ > SET_DECL_MODE (dp_ddecl, DECL_MODE (dead_param)); > m_dead_ssa_debug_equiv.put (parm_ddef, dp_ddecl); > } >@@ -2217,11 +2215,10 @@ ipa_param_body_adjustments::reset_debug_stmts () > gcc_assert (is_gimple_debug (stmt)); > if (vexpr == NULL && gsip != NULL) > { >- vexpr = make_node (DEBUG_EXPR_DECL); >- def_temp = gimple_build_debug_source_bind (vexpr, decl, NULL); >- DECL_ARTIFICIAL (vexpr) = 1; >- TREE_TYPE (vexpr) = TREE_TYPE (name); >+ vexpr = build_debug_expr_decl (TREE_TYPE (name)); >+ /* FIXME: Is setting the mode really necessary? */ > SET_DECL_MODE (vexpr, DECL_MODE (decl)); >+ def_temp = gimple_build_debug_source_bind (vexpr, decl, NULL); > gsi_insert_before (gsip, def_temp, GSI_SAME_STMT); > } > if (vexpr) >diff --git a/gcc/tree-inline.c b/gcc/tree-inline.c >index 53d664ec2e4..8c108d8e4e7 100644 >--- a/gcc/tree-inline.c >+++ b/gcc/tree-inline.c >@@ -193,7 +193,6 @@ remap_ssa_name (tree name, copy_body_data *id) > && id->entry_bb == NULL > && single_succ_p (ENTRY_BLOCK_PTR_FOR_FN (cfun))) > { >- tree vexpr = make_node (DEBUG_EXPR_DECL); > gimple *def_temp; > gimple_stmt_iterator gsi; > tree val = SSA_NAME_VAR (name); >@@ -210,10 +209,10 @@ remap_ssa_name (tree name, copy_body_data *id) > n = id->decl_map->get (val); > if (n && TREE_CODE (*n) == DEBUG_EXPR_DECL) > return *n; >- def_temp = gimple_build_debug_source_bind (vexpr, val, NULL); >- DECL_ARTIFICIAL (vexpr) = 1; >- TREE_TYPE (vexpr) = TREE_TYPE (name); >+ tree vexpr = build_debug_expr_decl (TREE_TYPE (name)); >+ /* FIXME: Is setting the mode really necessary? */ > SET_DECL_MODE (vexpr, DECL_MODE (SSA_NAME_VAR (name))); >+ def_temp = gimple_build_debug_source_bind (vexpr, val, NULL); > gsi = gsi_after_labels (single_succ (ENTRY_BLOCK_PTR_FOR_FN (cfun))); > gsi_insert_before (&gsi, def_temp, GSI_SAME_STMT); > insert_decl_map (id, val, vexpr); >@@ -6450,9 +6449,8 @@ tree_function_versioning (tree old_decl, tree new_decl, > debug_args = decl_debug_args_insert (new_decl); > len = vec_safe_length (*debug_args); > } >- ddecl = make_node (DEBUG_EXPR_DECL); >- DECL_ARTIFICIAL (ddecl) = 1; >- TREE_TYPE (ddecl) = TREE_TYPE (parm); >+ ddecl = build_debug_expr_decl (TREE_TYPE (parm)); >+ /* FIXME: Is setting the mode really necessary? */ > SET_DECL_MODE (ddecl, DECL_MODE (parm)); > vec_safe_push (*debug_args, DECL_ORIGIN (parm)); > vec_safe_push (*debug_args, ddecl); >@@ -6488,9 +6486,8 @@ tree_function_versioning (tree old_decl, tree new_decl, > vexpr = *d; > if (!vexpr) > { >- vexpr = make_node (DEBUG_EXPR_DECL); >- DECL_ARTIFICIAL (vexpr) = 1; >- TREE_TYPE (vexpr) = TREE_TYPE (parm); >+ vexpr = build_debug_expr_decl (TREE_TYPE (parm)); >+ /* FIXME: Is setting the mode really necessary? */ > SET_DECL_MODE (vexpr, DECL_MODE (parm)); > } > def_temp = gimple_build_debug_bind (var, vexpr, NULL); >diff --git a/gcc/tree-into-ssa.c b/gcc/tree-into-ssa.c >index 8045e34df26..265dcc5d42f 100644 >--- a/gcc/tree-into-ssa.c >+++ b/gcc/tree-into-ssa.c >@@ -1284,11 +1284,10 @@ rewrite_debug_stmt_uses (gimple *stmt) > if (def == NULL_TREE) > { > gimple *def_temp; >- def = make_node (DEBUG_EXPR_DECL); >- def_temp = gimple_build_debug_source_bind (def, var, NULL); >- DECL_ARTIFICIAL (def) = 1; >- TREE_TYPE (def) = TREE_TYPE (var); >+ def = build_debug_expr_decl (TREE_TYPE (var)); >+ /* FIXME: Is setting the mode really necessary? */ > SET_DECL_MODE (def, DECL_MODE (var)); >+ def_temp = gimple_build_debug_source_bind (def, var, NULL); > gsi = > gsi_after_labels (single_succ (ENTRY_BLOCK_PTR_FOR_FN (cfun))); > gsi_insert_before (&gsi, def_temp, GSI_SAME_STMT); >diff --git a/gcc/tree-ssa-loop-ivopts.c b/gcc/tree-ssa-loop-ivopts.c >index 4a498abe3b0..5a7fd305d91 100644 >--- a/gcc/tree-ssa-loop-ivopts.c >+++ b/gcc/tree-ssa-loop-ivopts.c >@@ -7742,9 +7742,8 @@ remove_unused_ivs (struct ivopts_data *data, bitmap toremove) > comp = unshare_expr (comp); > if (count > 1) > { >- tree vexpr = make_node (DEBUG_EXPR_DECL); >- DECL_ARTIFICIAL (vexpr) = 1; >- TREE_TYPE (vexpr) = TREE_TYPE (comp); >+ tree vexpr = build_debug_expr_decl (TREE_TYPE (comp)); >+ /* FIXME: Is setting the mode really necessary? */ > if (SSA_NAME_VAR (def)) > SET_DECL_MODE (vexpr, DECL_MODE (SSA_NAME_VAR (def))); > else >diff --git a/gcc/tree-ssa.c b/gcc/tree-ssa.c >index 3f25d654d3f..1565e21d983 100644 >--- a/gcc/tree-ssa.c >+++ b/gcc/tree-ssa.c >@@ -434,14 +434,13 @@ insert_debug_temp_for_var_def (gimple_stmt_iterator *gsi, tree var) > else > { > gdebug *def_temp; >- tree vexpr = make_node (DEBUG_EXPR_DECL); >+ tree vexpr = build_debug_expr_decl (TREE_TYPE (value)); > > def_temp = gimple_build_debug_bind (vexpr, > unshare_expr (value), > def_stmt); > >- DECL_ARTIFICIAL (vexpr) = 1; >- TREE_TYPE (vexpr) = TREE_TYPE (value); >+ /* FIXME: Is setting the mode really necessary? */ > if (DECL_P (value)) > SET_DECL_MODE (vexpr, DECL_MODE (value)); > else
diff --git a/gcc/cfgexpand.c b/gcc/cfgexpand.c index 55ff75bd78e..eb6466f4be6 100644 --- a/gcc/cfgexpand.c +++ b/gcc/cfgexpand.c @@ -5898,18 +5898,17 @@ expand_gimple_basic_block (basic_block bb, bool disable_tail_calls) temporary. */ gimple *debugstmt; tree value = gimple_assign_rhs_to_tree (def); - tree vexpr = make_node (DEBUG_EXPR_DECL); + tree vexpr = build_debug_expr_decl (TREE_TYPE (value)); rtx val; machine_mode mode; set_curr_insn_location (gimple_location (def)); - DECL_ARTIFICIAL (vexpr) = 1; - TREE_TYPE (vexpr) = TREE_TYPE (value); if (DECL_P (value)) mode = DECL_MODE (value); else mode = TYPE_MODE (TREE_TYPE (value)); + /* FIXME: Is setting the mode really necessary? */ SET_DECL_MODE (vexpr, mode); val = gen_rtx_VAR_LOCATION diff --git a/gcc/ipa-param-manipulation.c b/gcc/ipa-param-manipulation.c index ae3149718ca..a230735d71e 100644 --- a/gcc/ipa-param-manipulation.c +++ b/gcc/ipa-param-manipulation.c @@ -831,9 +831,8 @@ ipa_param_adjustments::modify_call (cgraph_edge *cs, } if (ddecl == NULL) { - ddecl = make_node (DEBUG_EXPR_DECL); - DECL_ARTIFICIAL (ddecl) = 1; - TREE_TYPE (ddecl) = TREE_TYPE (origin); + ddecl = build_debug_expr_decl (TREE_TYPE (origin)); + /* FIXME: Is setting the mode really necessary? */ SET_DECL_MODE (ddecl, DECL_MODE (origin)); vec_safe_push (*debug_args, origin); @@ -1063,9 +1062,8 @@ ipa_param_body_adjustments::mark_dead_statements (tree dead_param, return; } - tree dp_ddecl = make_node (DEBUG_EXPR_DECL); - DECL_ARTIFICIAL (dp_ddecl) = 1; - TREE_TYPE (dp_ddecl) = TREE_TYPE (dead_param); + tree dp_ddecl = build_debug_expr_decl (TREE_TYPE (dead_param)); + /* FIXME: Is setting the mode really necessary? */ SET_DECL_MODE (dp_ddecl, DECL_MODE (dead_param)); m_dead_ssa_debug_equiv.put (parm_ddef, dp_ddecl); } @@ -2217,11 +2215,10 @@ ipa_param_body_adjustments::reset_debug_stmts () gcc_assert (is_gimple_debug (stmt)); if (vexpr == NULL && gsip != NULL) { - vexpr = make_node (DEBUG_EXPR_DECL); - def_temp = gimple_build_debug_source_bind (vexpr, decl, NULL); - DECL_ARTIFICIAL (vexpr) = 1; - TREE_TYPE (vexpr) = TREE_TYPE (name); + vexpr = build_debug_expr_decl (TREE_TYPE (name)); + /* FIXME: Is setting the mode really necessary? */ SET_DECL_MODE (vexpr, DECL_MODE (decl)); + def_temp = gimple_build_debug_source_bind (vexpr, decl, NULL); gsi_insert_before (gsip, def_temp, GSI_SAME_STMT); } if (vexpr) diff --git a/gcc/tree-inline.c b/gcc/tree-inline.c index 53d664ec2e4..8c108d8e4e7 100644 --- a/gcc/tree-inline.c +++ b/gcc/tree-inline.c @@ -193,7 +193,6 @@ remap_ssa_name (tree name, copy_body_data *id) && id->entry_bb == NULL && single_succ_p (ENTRY_BLOCK_PTR_FOR_FN (cfun))) { - tree vexpr = make_node (DEBUG_EXPR_DECL); gimple *def_temp; gimple_stmt_iterator gsi; tree val = SSA_NAME_VAR (name); @@ -210,10 +209,10 @@ remap_ssa_name (tree name, copy_body_data *id) n = id->decl_map->get (val); if (n && TREE_CODE (*n) == DEBUG_EXPR_DECL) return *n; - def_temp = gimple_build_debug_source_bind (vexpr, val, NULL); - DECL_ARTIFICIAL (vexpr) = 1; - TREE_TYPE (vexpr) = TREE_TYPE (name); + tree vexpr = build_debug_expr_decl (TREE_TYPE (name)); + /* FIXME: Is setting the mode really necessary? */ SET_DECL_MODE (vexpr, DECL_MODE (SSA_NAME_VAR (name))); + def_temp = gimple_build_debug_source_bind (vexpr, val, NULL); gsi = gsi_after_labels (single_succ (ENTRY_BLOCK_PTR_FOR_FN (cfun))); gsi_insert_before (&gsi, def_temp, GSI_SAME_STMT); insert_decl_map (id, val, vexpr); @@ -6450,9 +6449,8 @@ tree_function_versioning (tree old_decl, tree new_decl, debug_args = decl_debug_args_insert (new_decl); len = vec_safe_length (*debug_args); } - ddecl = make_node (DEBUG_EXPR_DECL); - DECL_ARTIFICIAL (ddecl) = 1; - TREE_TYPE (ddecl) = TREE_TYPE (parm); + ddecl = build_debug_expr_decl (TREE_TYPE (parm)); + /* FIXME: Is setting the mode really necessary? */ SET_DECL_MODE (ddecl, DECL_MODE (parm)); vec_safe_push (*debug_args, DECL_ORIGIN (parm)); vec_safe_push (*debug_args, ddecl); @@ -6488,9 +6486,8 @@ tree_function_versioning (tree old_decl, tree new_decl, vexpr = *d; if (!vexpr) { - vexpr = make_node (DEBUG_EXPR_DECL); - DECL_ARTIFICIAL (vexpr) = 1; - TREE_TYPE (vexpr) = TREE_TYPE (parm); + vexpr = build_debug_expr_decl (TREE_TYPE (parm)); + /* FIXME: Is setting the mode really necessary? */ SET_DECL_MODE (vexpr, DECL_MODE (parm)); } def_temp = gimple_build_debug_bind (var, vexpr, NULL); diff --git a/gcc/tree-into-ssa.c b/gcc/tree-into-ssa.c index 8045e34df26..265dcc5d42f 100644 --- a/gcc/tree-into-ssa.c +++ b/gcc/tree-into-ssa.c @@ -1284,11 +1284,10 @@ rewrite_debug_stmt_uses (gimple *stmt) if (def == NULL_TREE) { gimple *def_temp; - def = make_node (DEBUG_EXPR_DECL); - def_temp = gimple_build_debug_source_bind (def, var, NULL); - DECL_ARTIFICIAL (def) = 1; - TREE_TYPE (def) = TREE_TYPE (var); + def = build_debug_expr_decl (TREE_TYPE (var)); + /* FIXME: Is setting the mode really necessary? */ SET_DECL_MODE (def, DECL_MODE (var)); + def_temp = gimple_build_debug_source_bind (def, var, NULL); gsi = gsi_after_labels (single_succ (ENTRY_BLOCK_PTR_FOR_FN (cfun))); gsi_insert_before (&gsi, def_temp, GSI_SAME_STMT); diff --git a/gcc/tree-ssa-loop-ivopts.c b/gcc/tree-ssa-loop-ivopts.c index 4a498abe3b0..5a7fd305d91 100644 --- a/gcc/tree-ssa-loop-ivopts.c +++ b/gcc/tree-ssa-loop-ivopts.c @@ -7742,9 +7742,8 @@ remove_unused_ivs (struct ivopts_data *data, bitmap toremove) comp = unshare_expr (comp); if (count > 1) { - tree vexpr = make_node (DEBUG_EXPR_DECL); - DECL_ARTIFICIAL (vexpr) = 1; - TREE_TYPE (vexpr) = TREE_TYPE (comp); + tree vexpr = build_debug_expr_decl (TREE_TYPE (comp)); + /* FIXME: Is setting the mode really necessary? */ if (SSA_NAME_VAR (def)) SET_DECL_MODE (vexpr, DECL_MODE (SSA_NAME_VAR (def))); else diff --git a/gcc/tree-ssa.c b/gcc/tree-ssa.c index 3f25d654d3f..1565e21d983 100644 --- a/gcc/tree-ssa.c +++ b/gcc/tree-ssa.c @@ -434,14 +434,13 @@ insert_debug_temp_for_var_def (gimple_stmt_iterator *gsi, tree var) else { gdebug *def_temp; - tree vexpr = make_node (DEBUG_EXPR_DECL); + tree vexpr = build_debug_expr_decl (TREE_TYPE (value)); def_temp = gimple_build_debug_bind (vexpr, unshare_expr (value), def_stmt); - DECL_ARTIFICIAL (vexpr) = 1; - TREE_TYPE (vexpr) = TREE_TYPE (value); + /* FIXME: Is setting the mode really necessary? */ if (DECL_P (value)) SET_DECL_MODE (vexpr, DECL_MODE (value)); else