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?

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.

Thanks,

Martin


gcc/ChangeLog:

2021-11-11  Martin Jambor  <mjam...@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
-- 
2.33.0

Reply via email to