[PATCH] Introduce build_debug_expr_decl

[PATCH] Introduce build_debug_expr_decl

[Martin Jambor]

Hi,

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?

Thanks,

Martin


gcc/ChangeLog:

2021-11-08  Martin Jambor  <mjambor@suse.cz>

	* tree.h (build_debug_expr_decl): Declare.
	* tree.c (build_debug_expr_decl): New function.
	* cfgexpand.c (avoid_deep_ter_for_debug): Use build_debug_expr_decl
	instead of building a DEBUG_EXPR_DECL.
	* ipa-param-manipulation.c
	(ipa_param_body_adjustments::prepare_debug_expressions): Likewise.
	* omp-simd-clone.c (ipa_simd_modify_stmt_ops): Likewise.
	* tree-ssa-ccp.c (optimize_atomic_bit_test_and): Likewise.
	* tree-ssa-phiopt.c (spaceship_replacement): Likewise.
	* tree-ssa-reassoc.c (make_new_ssa_for_def): Likewise.
---
 gcc/cfgexpand.c              |  5 +----
 gcc/ipa-param-manipulation.c |  5 +----
 gcc/omp-simd-clone.c         |  5 +----
 gcc/tree-ssa-ccp.c           |  5 +----
 gcc/tree-ssa-phiopt.c        | 10 ++--------
 gcc/tree-ssa-reassoc.c       |  5 +----
 gcc/tree.c                   | 12 ++++++++++++
 gcc/tree.h                   |  1 +
 8 files changed, 20 insertions(+), 28 deletions(-)

diff --git a/gcc/cfgexpand.c b/gcc/cfgexpand.c
index 01d0cdc548a..55ff75bd78e 100644
--- a/gcc/cfgexpand.c
+++ b/gcc/cfgexpand.c
@@ -4341,11 +4341,8 @@ avoid_deep_ter_for_debug (gimple *stmt, int depth)
 	  tree &vexpr = deep_ter_debug_map->get_or_insert (use);
 	  if (vexpr != NULL)
 	    continue;
-	  vexpr = make_node (DEBUG_EXPR_DECL);
+	  vexpr = build_debug_expr_decl (TREE_TYPE (use));
 	  gimple *def_temp = gimple_build_debug_bind (vexpr, use, g);
-	  DECL_ARTIFICIAL (vexpr) = 1;
-	  TREE_TYPE (vexpr) = TREE_TYPE (use);
-	  SET_DECL_MODE (vexpr, TYPE_MODE (TREE_TYPE (use)));
 	  gimple_stmt_iterator gsi = gsi_for_stmt (g);
 	  gsi_insert_after (&gsi, def_temp, GSI_NEW_STMT);
 	  avoid_deep_ter_for_debug (def_temp, 0);
diff --git a/gcc/ipa-param-manipulation.c b/gcc/ipa-param-manipulation.c
index 4610fc4ac03..ae3149718ca 100644
--- a/gcc/ipa-param-manipulation.c
+++ b/gcc/ipa-param-manipulation.c
@@ -1200,10 +1200,7 @@ ipa_param_body_adjustments::prepare_debug_expressions (tree dead_ssa)
 	= unshare_expr_without_location (gimple_assign_rhs_to_tree (def));
       remap_with_debug_expressions (&val);
 
-      tree vexpr = make_node (DEBUG_EXPR_DECL);
-      DECL_ARTIFICIAL (vexpr) = 1;
-      TREE_TYPE (vexpr) = TREE_TYPE (val);
-      SET_DECL_MODE (vexpr, TYPE_MODE (TREE_TYPE (val)));
+      tree vexpr = build_debug_expr_decl (TREE_TYPE (val));
       m_dead_stmt_debug_equiv.put (def, val);
       m_dead_ssa_debug_equiv.put (dead_ssa, vexpr);
       return true;
diff --git a/gcc/omp-simd-clone.c b/gcc/omp-simd-clone.c
index b772b7ff520..4d43a86669a 100644
--- a/gcc/omp-simd-clone.c
+++ b/gcc/omp-simd-clone.c
@@ -910,11 +910,8 @@ ipa_simd_modify_stmt_ops (tree *tp, int *walk_subtrees, void *data)
       gimple *stmt;
       if (is_gimple_debug (info->stmt))
 	{
-	  tree vexpr = make_node (DEBUG_EXPR_DECL);
+	  tree vexpr = build_debug_expr_decl (TREE_TYPE (repl));
 	  stmt = gimple_build_debug_source_bind (vexpr, repl, NULL);
-	  DECL_ARTIFICIAL (vexpr) = 1;
-	  TREE_TYPE (vexpr) = TREE_TYPE (repl);
-	  SET_DECL_MODE (vexpr, TYPE_MODE (TREE_TYPE (repl)));
 	  repl = vexpr;
 	}
       else
diff --git a/gcc/tree-ssa-ccp.c b/gcc/tree-ssa-ccp.c
index 70ce6a4d5b8..60ae5e6601f 100644
--- a/gcc/tree-ssa-ccp.c
+++ b/gcc/tree-ssa-ccp.c
@@ -3452,10 +3452,7 @@ optimize_atomic_bit_test_and (gimple_stmt_iterator *gsip,
       tree temp = NULL_TREE;
       if (!throws || after || single_pred_p (e->dest))
 	{
-	  temp = make_node (DEBUG_EXPR_DECL);
-	  DECL_ARTIFICIAL (temp) = 1;
-	  TREE_TYPE (temp) = TREE_TYPE (lhs);
-	  SET_DECL_MODE (temp, TYPE_MODE (TREE_TYPE (lhs)));
+	  temp = build_debug_expr_decl (TREE_TYPE (lhs));
 	  tree t = build2 (LSHIFT_EXPR, TREE_TYPE (lhs), new_lhs, bit);
 	  g = gimple_build_debug_bind (temp, t, g);
 	  if (throws && !after)
diff --git a/gcc/tree-ssa-phiopt.c b/gcc/tree-ssa-phiopt.c
index 0e339c46afa..173ac835ca6 100644
--- a/gcc/tree-ssa-phiopt.c
+++ b/gcc/tree-ssa-phiopt.c
@@ -2429,19 +2429,13 @@ spaceship_replacement (basic_block cond_bb, basic_block middle_bb,
 	     all floating point numbers should be comparable.  */
 	  gimple_stmt_iterator gsi = gsi_after_labels (gimple_bb (phi));
 	  tree type = TREE_TYPE (phires);
-	  tree temp1 = make_node (DEBUG_EXPR_DECL);
-	  DECL_ARTIFICIAL (temp1) = 1;
-	  TREE_TYPE (temp1) = type;
-	  SET_DECL_MODE (temp1, TYPE_MODE (type));
+	  tree temp1 = build_debug_expr_decl (type);
 	  tree t = build2 (one_cmp, boolean_type_node, lhs1, rhs2);
 	  t = build3 (COND_EXPR, type, t, build_one_cst (type),
 		      build_int_cst (type, -1));
 	  gimple *g = gimple_build_debug_bind (temp1, t, phi);
 	  gsi_insert_before (&gsi, g, GSI_SAME_STMT);
-	  tree temp2 = make_node (DEBUG_EXPR_DECL);
-	  DECL_ARTIFICIAL (temp2) = 1;
-	  TREE_TYPE (temp2) = type;
-	  SET_DECL_MODE (temp2, TYPE_MODE (type));
+	  tree temp2 = build_debug_expr_decl (type);
 	  t = build2 (EQ_EXPR, boolean_type_node, lhs1, rhs2);
 	  t = build3 (COND_EXPR, type, t, build_zero_cst (type), temp1);
 	  g = gimple_build_debug_bind (temp2, t, phi);
diff --git a/gcc/tree-ssa-reassoc.c b/gcc/tree-ssa-reassoc.c
index db9fb4e1cac..6a555e7c553 100644
--- a/gcc/tree-ssa-reassoc.c
+++ b/gcc/tree-ssa-reassoc.c
@@ -1259,15 +1259,12 @@ make_new_ssa_for_def (gimple *stmt, enum tree_code opcode, tree op)
 	{
 	  if (new_debug_lhs == NULL_TREE)
 	    {
-	      new_debug_lhs = make_node (DEBUG_EXPR_DECL);
+	      new_debug_lhs = build_debug_expr_decl (TREE_TYPE (lhs));
 	      gdebug *def_temp
 		= gimple_build_debug_bind (new_debug_lhs,
 					   build2 (opcode, TREE_TYPE (lhs),
 						   new_lhs, op),
 					   stmt);
-	      DECL_ARTIFICIAL (new_debug_lhs) = 1;
-	      TREE_TYPE (new_debug_lhs) = TREE_TYPE (lhs);
-	      SET_DECL_MODE (new_debug_lhs, TYPE_MODE (TREE_TYPE (lhs)));
 	      gimple_set_uid (def_temp, gimple_uid (stmt));
 	      gimple_stmt_iterator gsi = gsi_for_stmt (stmt);
 	      gsi_insert_after (&gsi, def_temp, GSI_SAME_STMT);
diff --git a/gcc/tree.c b/gcc/tree.c
index 7bfd64160f4..845228a055b 100644
--- a/gcc/tree.c
+++ b/gcc/tree.c
@@ -5280,6 +5280,18 @@ build_decl (location_t loc, enum tree_code code, tree name,
   return t;
 }
 
+/* Create and return a DEBUG_EXPR_DECL node of the given TYPE.  */
+
+tree
+build_debug_expr_decl (tree type)
+{
+  tree vexpr = make_node (DEBUG_EXPR_DECL);
+  DECL_ARTIFICIAL (vexpr) = 1;
+  TREE_TYPE (vexpr) = type;
+  SET_DECL_MODE (vexpr, TYPE_MODE (type));
+  return vexpr;
+}
+
 /* Builds and returns function declaration with NAME and TYPE.  */
 
 tree
diff --git a/gcc/tree.h b/gcc/tree.h
index 7542d97ce12..f62c00bc870 100644
--- a/gcc/tree.h
+++ b/gcc/tree.h
@@ -4567,6 +4567,7 @@ extern tree build_tree_list (tree, tree CXX_MEM_STAT_INFO);
 extern tree build_tree_list_vec (const vec<tree, va_gc> * CXX_MEM_STAT_INFO);
 extern tree build_decl (location_t, enum tree_code,
 			tree, tree CXX_MEM_STAT_INFO);
+extern tree build_debug_expr_decl (tree type);
 extern tree build_fn_decl (const char *, tree);
 extern tree build_translation_unit_decl (tree);
 extern tree build_block (tree, tree, tree, tree);
-- 
2.33.0

Re: [PATCH] Introduce build_debug_expr_decl

[Prathamesh Kulkarni]

On Mon, 8 Nov 2021 at 23:24, Martin Jambor <mjambor@suse.cz> wrote:
>
> Hi,
>
> 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?
>
> Thanks,
>
> Martin
>
>
> gcc/ChangeLog:
>
> 2021-11-08  Martin Jambor  <mjambor@suse.cz>
>
>         * tree.h (build_debug_expr_decl): Declare.
>         * tree.c (build_debug_expr_decl): New function.
>         * cfgexpand.c (avoid_deep_ter_for_debug): Use build_debug_expr_decl
>         instead of building a DEBUG_EXPR_DECL.
>         * ipa-param-manipulation.c
>         (ipa_param_body_adjustments::prepare_debug_expressions): Likewise.
>         * omp-simd-clone.c (ipa_simd_modify_stmt_ops): Likewise.
>         * tree-ssa-ccp.c (optimize_atomic_bit_test_and): Likewise.
>         * tree-ssa-phiopt.c (spaceship_replacement): Likewise.
>         * tree-ssa-reassoc.c (make_new_ssa_for_def): Likewise.
> ---
>  gcc/cfgexpand.c              |  5 +----
>  gcc/ipa-param-manipulation.c |  5 +----
>  gcc/omp-simd-clone.c         |  5 +----
>  gcc/tree-ssa-ccp.c           |  5 +----
>  gcc/tree-ssa-phiopt.c        | 10 ++--------
>  gcc/tree-ssa-reassoc.c       |  5 +----
>  gcc/tree.c                   | 12 ++++++++++++
>  gcc/tree.h                   |  1 +
>  8 files changed, 20 insertions(+), 28 deletions(-)
>
> diff --git a/gcc/cfgexpand.c b/gcc/cfgexpand.c
> index 01d0cdc548a..55ff75bd78e 100644
> --- a/gcc/cfgexpand.c
> +++ b/gcc/cfgexpand.c
> @@ -4341,11 +4341,8 @@ avoid_deep_ter_for_debug (gimple *stmt, int depth)
>           tree &vexpr = deep_ter_debug_map->get_or_insert (use);
>           if (vexpr != NULL)
>             continue;
> -         vexpr = make_node (DEBUG_EXPR_DECL);
> +         vexpr = build_debug_expr_decl (TREE_TYPE (use));
>           gimple *def_temp = gimple_build_debug_bind (vexpr, use, g);
> -         DECL_ARTIFICIAL (vexpr) = 1;
> -         TREE_TYPE (vexpr) = TREE_TYPE (use);
> -         SET_DECL_MODE (vexpr, TYPE_MODE (TREE_TYPE (use)));
>           gimple_stmt_iterator gsi = gsi_for_stmt (g);
>           gsi_insert_after (&gsi, def_temp, GSI_NEW_STMT);
>           avoid_deep_ter_for_debug (def_temp, 0);
> diff --git a/gcc/ipa-param-manipulation.c b/gcc/ipa-param-manipulation.c
> index 4610fc4ac03..ae3149718ca 100644
> --- a/gcc/ipa-param-manipulation.c
> +++ b/gcc/ipa-param-manipulation.c
> @@ -1200,10 +1200,7 @@ ipa_param_body_adjustments::prepare_debug_expressions (tree dead_ssa)
>         = unshare_expr_without_location (gimple_assign_rhs_to_tree (def));
>        remap_with_debug_expressions (&val);
>
> -      tree vexpr = make_node (DEBUG_EXPR_DECL);
> -      DECL_ARTIFICIAL (vexpr) = 1;
> -      TREE_TYPE (vexpr) = TREE_TYPE (val);
> -      SET_DECL_MODE (vexpr, TYPE_MODE (TREE_TYPE (val)));
> +      tree vexpr = build_debug_expr_decl (TREE_TYPE (val));
>        m_dead_stmt_debug_equiv.put (def, val);
>        m_dead_ssa_debug_equiv.put (dead_ssa, vexpr);
>        return true;
> diff --git a/gcc/omp-simd-clone.c b/gcc/omp-simd-clone.c
> index b772b7ff520..4d43a86669a 100644
> --- a/gcc/omp-simd-clone.c
> +++ b/gcc/omp-simd-clone.c
> @@ -910,11 +910,8 @@ ipa_simd_modify_stmt_ops (tree *tp, int *walk_subtrees, void *data)
>        gimple *stmt;
>        if (is_gimple_debug (info->stmt))
>         {
> -         tree vexpr = make_node (DEBUG_EXPR_DECL);
> +         tree vexpr = build_debug_expr_decl (TREE_TYPE (repl));
>           stmt = gimple_build_debug_source_bind (vexpr, repl, NULL);
> -         DECL_ARTIFICIAL (vexpr) = 1;
> -         TREE_TYPE (vexpr) = TREE_TYPE (repl);
> -         SET_DECL_MODE (vexpr, TYPE_MODE (TREE_TYPE (repl)));
>           repl = vexpr;
>         }
>        else
> diff --git a/gcc/tree-ssa-ccp.c b/gcc/tree-ssa-ccp.c
> index 70ce6a4d5b8..60ae5e6601f 100644
> --- a/gcc/tree-ssa-ccp.c
> +++ b/gcc/tree-ssa-ccp.c
> @@ -3452,10 +3452,7 @@ optimize_atomic_bit_test_and (gimple_stmt_iterator *gsip,
>        tree temp = NULL_TREE;
>        if (!throws || after || single_pred_p (e->dest))
>         {
> -         temp = make_node (DEBUG_EXPR_DECL);
> -         DECL_ARTIFICIAL (temp) = 1;
> -         TREE_TYPE (temp) = TREE_TYPE (lhs);
> -         SET_DECL_MODE (temp, TYPE_MODE (TREE_TYPE (lhs)));
> +         temp = build_debug_expr_decl (TREE_TYPE (lhs));
>           tree t = build2 (LSHIFT_EXPR, TREE_TYPE (lhs), new_lhs, bit);
>           g = gimple_build_debug_bind (temp, t, g);
>           if (throws && !after)
> diff --git a/gcc/tree-ssa-phiopt.c b/gcc/tree-ssa-phiopt.c
> index 0e339c46afa..173ac835ca6 100644
> --- a/gcc/tree-ssa-phiopt.c
> +++ b/gcc/tree-ssa-phiopt.c
> @@ -2429,19 +2429,13 @@ spaceship_replacement (basic_block cond_bb, basic_block middle_bb,
>              all floating point numbers should be comparable.  */
>           gimple_stmt_iterator gsi = gsi_after_labels (gimple_bb (phi));
>           tree type = TREE_TYPE (phires);
> -         tree temp1 = make_node (DEBUG_EXPR_DECL);
> -         DECL_ARTIFICIAL (temp1) = 1;
> -         TREE_TYPE (temp1) = type;
> -         SET_DECL_MODE (temp1, TYPE_MODE (type));
> +         tree temp1 = build_debug_expr_decl (type);
>           tree t = build2 (one_cmp, boolean_type_node, lhs1, rhs2);
>           t = build3 (COND_EXPR, type, t, build_one_cst (type),
>                       build_int_cst (type, -1));
>           gimple *g = gimple_build_debug_bind (temp1, t, phi);
>           gsi_insert_before (&gsi, g, GSI_SAME_STMT);
> -         tree temp2 = make_node (DEBUG_EXPR_DECL);
> -         DECL_ARTIFICIAL (temp2) = 1;
> -         TREE_TYPE (temp2) = type;
> -         SET_DECL_MODE (temp2, TYPE_MODE (type));
> +         tree temp2 = build_debug_expr_decl (type);
>           t = build2 (EQ_EXPR, boolean_type_node, lhs1, rhs2);
>           t = build3 (COND_EXPR, type, t, build_zero_cst (type), temp1);
>           g = gimple_build_debug_bind (temp2, t, phi);
> diff --git a/gcc/tree-ssa-reassoc.c b/gcc/tree-ssa-reassoc.c
> index db9fb4e1cac..6a555e7c553 100644
> --- a/gcc/tree-ssa-reassoc.c
> +++ b/gcc/tree-ssa-reassoc.c
> @@ -1259,15 +1259,12 @@ make_new_ssa_for_def (gimple *stmt, enum tree_code opcode, tree op)
>         {
>           if (new_debug_lhs == NULL_TREE)
>             {
> -             new_debug_lhs = make_node (DEBUG_EXPR_DECL);
> +             new_debug_lhs = build_debug_expr_decl (TREE_TYPE (lhs));
>               gdebug *def_temp
>                 = gimple_build_debug_bind (new_debug_lhs,
>                                            build2 (opcode, TREE_TYPE (lhs),
>                                                    new_lhs, op),
>                                            stmt);
> -             DECL_ARTIFICIAL (new_debug_lhs) = 1;
> -             TREE_TYPE (new_debug_lhs) = TREE_TYPE (lhs);
> -             SET_DECL_MODE (new_debug_lhs, TYPE_MODE (TREE_TYPE (lhs)));
>               gimple_set_uid (def_temp, gimple_uid (stmt));
>               gimple_stmt_iterator gsi = gsi_for_stmt (stmt);
>               gsi_insert_after (&gsi, def_temp, GSI_SAME_STMT);
> diff --git a/gcc/tree.c b/gcc/tree.c
> index 7bfd64160f4..845228a055b 100644
> --- a/gcc/tree.c
> +++ b/gcc/tree.c
> @@ -5280,6 +5280,18 @@ build_decl (location_t loc, enum tree_code code, tree name,
>    return t;
>  }
>
> +/* Create and return a DEBUG_EXPR_DECL node of the given TYPE.  */
> +
> +tree
> +build_debug_expr_decl (tree type)
Hi, sorry to nitpick, but would it be slightly better to use
const_tree type instead ?

Thanks,
Prathamesh
> +{
> +  tree vexpr = make_node (DEBUG_EXPR_DECL);
> +  DECL_ARTIFICIAL (vexpr) = 1;
> +  TREE_TYPE (vexpr) = type;
> +  SET_DECL_MODE (vexpr, TYPE_MODE (type));
> +  return vexpr;
> +}
> +
>  /* Builds and returns function declaration with NAME and TYPE.  */
>
>  tree
> diff --git a/gcc/tree.h b/gcc/tree.h
> index 7542d97ce12..f62c00bc870 100644
> --- a/gcc/tree.h
> +++ b/gcc/tree.h
> @@ -4567,6 +4567,7 @@ extern tree build_tree_list (tree, tree CXX_MEM_STAT_INFO);
>  extern tree build_tree_list_vec (const vec<tree, va_gc> * CXX_MEM_STAT_INFO);
>  extern tree build_decl (location_t, enum tree_code,
>                         tree, tree CXX_MEM_STAT_INFO);
> +extern tree build_debug_expr_decl (tree type);
>  extern tree build_fn_decl (const char *, tree);
>  extern tree build_translation_unit_decl (tree);
>  extern tree build_block (tree, tree, tree, tree);
> --
> 2.33.0
>

Re: [PATCH] Introduce build_debug_expr_decl

[Richard Biener]

On Mon, 8 Nov 2021, Martin Jambor wrote:

> Hi,
> 
> 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?

Richard.

> Thanks,
> 
> Martin
> 
> 
> gcc/ChangeLog:
> 
> 2021-11-08  Martin Jambor  <mjambor@suse.cz>
> 
> 	* tree.h (build_debug_expr_decl): Declare.
> 	* tree.c (build_debug_expr_decl): New function.
> 	* cfgexpand.c (avoid_deep_ter_for_debug): Use build_debug_expr_decl
> 	instead of building a DEBUG_EXPR_DECL.
> 	* ipa-param-manipulation.c
> 	(ipa_param_body_adjustments::prepare_debug_expressions): Likewise.
> 	* omp-simd-clone.c (ipa_simd_modify_stmt_ops): Likewise.
> 	* tree-ssa-ccp.c (optimize_atomic_bit_test_and): Likewise.
> 	* tree-ssa-phiopt.c (spaceship_replacement): Likewise.
> 	* tree-ssa-reassoc.c (make_new_ssa_for_def): Likewise.
> ---
>  gcc/cfgexpand.c              |  5 +----
>  gcc/ipa-param-manipulation.c |  5 +----
>  gcc/omp-simd-clone.c         |  5 +----
>  gcc/tree-ssa-ccp.c           |  5 +----
>  gcc/tree-ssa-phiopt.c        | 10 ++--------
>  gcc/tree-ssa-reassoc.c       |  5 +----
>  gcc/tree.c                   | 12 ++++++++++++
>  gcc/tree.h                   |  1 +
>  8 files changed, 20 insertions(+), 28 deletions(-)
> 
> diff --git a/gcc/cfgexpand.c b/gcc/cfgexpand.c
> index 01d0cdc548a..55ff75bd78e 100644
> --- a/gcc/cfgexpand.c
> +++ b/gcc/cfgexpand.c
> @@ -4341,11 +4341,8 @@ avoid_deep_ter_for_debug (gimple *stmt, int depth)
>  	  tree &vexpr = deep_ter_debug_map->get_or_insert (use);
>  	  if (vexpr != NULL)
>  	    continue;
> -	  vexpr = make_node (DEBUG_EXPR_DECL);
> +	  vexpr = build_debug_expr_decl (TREE_TYPE (use));
>  	  gimple *def_temp = gimple_build_debug_bind (vexpr, use, g);
> -	  DECL_ARTIFICIAL (vexpr) = 1;
> -	  TREE_TYPE (vexpr) = TREE_TYPE (use);
> -	  SET_DECL_MODE (vexpr, TYPE_MODE (TREE_TYPE (use)));
>  	  gimple_stmt_iterator gsi = gsi_for_stmt (g);
>  	  gsi_insert_after (&gsi, def_temp, GSI_NEW_STMT);
>  	  avoid_deep_ter_for_debug (def_temp, 0);
> diff --git a/gcc/ipa-param-manipulation.c b/gcc/ipa-param-manipulation.c
> index 4610fc4ac03..ae3149718ca 100644
> --- a/gcc/ipa-param-manipulation.c
> +++ b/gcc/ipa-param-manipulation.c
> @@ -1200,10 +1200,7 @@ ipa_param_body_adjustments::prepare_debug_expressions (tree dead_ssa)
>  	= unshare_expr_without_location (gimple_assign_rhs_to_tree (def));
>        remap_with_debug_expressions (&val);
>  
> -      tree vexpr = make_node (DEBUG_EXPR_DECL);
> -      DECL_ARTIFICIAL (vexpr) = 1;
> -      TREE_TYPE (vexpr) = TREE_TYPE (val);
> -      SET_DECL_MODE (vexpr, TYPE_MODE (TREE_TYPE (val)));
> +      tree vexpr = build_debug_expr_decl (TREE_TYPE (val));
>        m_dead_stmt_debug_equiv.put (def, val);
>        m_dead_ssa_debug_equiv.put (dead_ssa, vexpr);
>        return true;
> diff --git a/gcc/omp-simd-clone.c b/gcc/omp-simd-clone.c
> index b772b7ff520..4d43a86669a 100644
> --- a/gcc/omp-simd-clone.c
> +++ b/gcc/omp-simd-clone.c
> @@ -910,11 +910,8 @@ ipa_simd_modify_stmt_ops (tree *tp, int *walk_subtrees, void *data)
>        gimple *stmt;
>        if (is_gimple_debug (info->stmt))
>  	{
> -	  tree vexpr = make_node (DEBUG_EXPR_DECL);
> +	  tree vexpr = build_debug_expr_decl (TREE_TYPE (repl));
>  	  stmt = gimple_build_debug_source_bind (vexpr, repl, NULL);
> -	  DECL_ARTIFICIAL (vexpr) = 1;
> -	  TREE_TYPE (vexpr) = TREE_TYPE (repl);
> -	  SET_DECL_MODE (vexpr, TYPE_MODE (TREE_TYPE (repl)));
>  	  repl = vexpr;
>  	}
>        else
> diff --git a/gcc/tree-ssa-ccp.c b/gcc/tree-ssa-ccp.c
> index 70ce6a4d5b8..60ae5e6601f 100644
> --- a/gcc/tree-ssa-ccp.c
> +++ b/gcc/tree-ssa-ccp.c
> @@ -3452,10 +3452,7 @@ optimize_atomic_bit_test_and (gimple_stmt_iterator *gsip,
>        tree temp = NULL_TREE;
>        if (!throws || after || single_pred_p (e->dest))
>  	{
> -	  temp = make_node (DEBUG_EXPR_DECL);
> -	  DECL_ARTIFICIAL (temp) = 1;
> -	  TREE_TYPE (temp) = TREE_TYPE (lhs);
> -	  SET_DECL_MODE (temp, TYPE_MODE (TREE_TYPE (lhs)));
> +	  temp = build_debug_expr_decl (TREE_TYPE (lhs));
>  	  tree t = build2 (LSHIFT_EXPR, TREE_TYPE (lhs), new_lhs, bit);
>  	  g = gimple_build_debug_bind (temp, t, g);
>  	  if (throws && !after)
> diff --git a/gcc/tree-ssa-phiopt.c b/gcc/tree-ssa-phiopt.c
> index 0e339c46afa..173ac835ca6 100644
> --- a/gcc/tree-ssa-phiopt.c
> +++ b/gcc/tree-ssa-phiopt.c
> @@ -2429,19 +2429,13 @@ spaceship_replacement (basic_block cond_bb, basic_block middle_bb,
>  	     all floating point numbers should be comparable.  */
>  	  gimple_stmt_iterator gsi = gsi_after_labels (gimple_bb (phi));
>  	  tree type = TREE_TYPE (phires);
> -	  tree temp1 = make_node (DEBUG_EXPR_DECL);
> -	  DECL_ARTIFICIAL (temp1) = 1;
> -	  TREE_TYPE (temp1) = type;
> -	  SET_DECL_MODE (temp1, TYPE_MODE (type));
> +	  tree temp1 = build_debug_expr_decl (type);
>  	  tree t = build2 (one_cmp, boolean_type_node, lhs1, rhs2);
>  	  t = build3 (COND_EXPR, type, t, build_one_cst (type),
>  		      build_int_cst (type, -1));
>  	  gimple *g = gimple_build_debug_bind (temp1, t, phi);
>  	  gsi_insert_before (&gsi, g, GSI_SAME_STMT);
> -	  tree temp2 = make_node (DEBUG_EXPR_DECL);
> -	  DECL_ARTIFICIAL (temp2) = 1;
> -	  TREE_TYPE (temp2) = type;
> -	  SET_DECL_MODE (temp2, TYPE_MODE (type));
> +	  tree temp2 = build_debug_expr_decl (type);
>  	  t = build2 (EQ_EXPR, boolean_type_node, lhs1, rhs2);
>  	  t = build3 (COND_EXPR, type, t, build_zero_cst (type), temp1);
>  	  g = gimple_build_debug_bind (temp2, t, phi);
> diff --git a/gcc/tree-ssa-reassoc.c b/gcc/tree-ssa-reassoc.c
> index db9fb4e1cac..6a555e7c553 100644
> --- a/gcc/tree-ssa-reassoc.c
> +++ b/gcc/tree-ssa-reassoc.c
> @@ -1259,15 +1259,12 @@ make_new_ssa_for_def (gimple *stmt, enum tree_code opcode, tree op)
>  	{
>  	  if (new_debug_lhs == NULL_TREE)
>  	    {
> -	      new_debug_lhs = make_node (DEBUG_EXPR_DECL);
> +	      new_debug_lhs = build_debug_expr_decl (TREE_TYPE (lhs));
>  	      gdebug *def_temp
>  		= gimple_build_debug_bind (new_debug_lhs,
>  					   build2 (opcode, TREE_TYPE (lhs),
>  						   new_lhs, op),
>  					   stmt);
> -	      DECL_ARTIFICIAL (new_debug_lhs) = 1;
> -	      TREE_TYPE (new_debug_lhs) = TREE_TYPE (lhs);
> -	      SET_DECL_MODE (new_debug_lhs, TYPE_MODE (TREE_TYPE (lhs)));
>  	      gimple_set_uid (def_temp, gimple_uid (stmt));
>  	      gimple_stmt_iterator gsi = gsi_for_stmt (stmt);
>  	      gsi_insert_after (&gsi, def_temp, GSI_SAME_STMT);
> diff --git a/gcc/tree.c b/gcc/tree.c
> index 7bfd64160f4..845228a055b 100644
> --- a/gcc/tree.c
> +++ b/gcc/tree.c
> @@ -5280,6 +5280,18 @@ build_decl (location_t loc, enum tree_code code, tree name,
>    return t;
>  }
>  
> +/* Create and return a DEBUG_EXPR_DECL node of the given TYPE.  */
> +
> +tree
> +build_debug_expr_decl (tree type)
> +{
> +  tree vexpr = make_node (DEBUG_EXPR_DECL);
> +  DECL_ARTIFICIAL (vexpr) = 1;
> +  TREE_TYPE (vexpr) = type;
> +  SET_DECL_MODE (vexpr, TYPE_MODE (type));
> +  return vexpr;
> +}
> +
>  /* Builds and returns function declaration with NAME and TYPE.  */
>  
>  tree
> diff --git a/gcc/tree.h b/gcc/tree.h
> index 7542d97ce12..f62c00bc870 100644
> --- a/gcc/tree.h
> +++ b/gcc/tree.h
> @@ -4567,6 +4567,7 @@ extern tree build_tree_list (tree, tree CXX_MEM_STAT_INFO);
>  extern tree build_tree_list_vec (const vec<tree, va_gc> * CXX_MEM_STAT_INFO);
>  extern tree build_decl (location_t, enum tree_code,
>  			tree, tree CXX_MEM_STAT_INFO);
> +extern tree build_debug_expr_decl (tree type);
>  extern tree build_fn_decl (const char *, tree);
>  extern tree build_translation_unit_decl (tree);
>  extern tree build_block (tree, tree, tree, tree);
> 

-- 
Richard Biener <rguenther@suse.de>
SUSE Software Solutions Germany GmbH, Maxfeldstrasse 5, 90409 Nuernberg,
Germany; GF: Ivo Totev; HRB 36809 (AG Nuernberg)

Re: [PATCH] Introduce build_debug_expr_decl

[Martin Jambor]

Hi,

On Mon, Nov 08 2021, Prathamesh Kulkarni wrote:
> On Mon, 8 Nov 2021 at 23:24, Martin Jambor <mjambor@suse.cz> wrote:
>>
>> Hi,
>>
>> 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?
>>
>> Thanks,
>>
>> Martin
>>
>>
>> gcc/ChangeLog:
>>
>> 2021-11-08  Martin Jambor  <mjambor@suse.cz>
>>
>>         * tree.h (build_debug_expr_decl): Declare.
>>         * tree.c (build_debug_expr_decl): New function.
>>         * cfgexpand.c (avoid_deep_ter_for_debug): Use build_debug_expr_decl
>>         instead of building a DEBUG_EXPR_DECL.
>>         * ipa-param-manipulation.c
>>         (ipa_param_body_adjustments::prepare_debug_expressions): Likewise.
>>         * omp-simd-clone.c (ipa_simd_modify_stmt_ops): Likewise.
>>         * tree-ssa-ccp.c (optimize_atomic_bit_test_and): Likewise.
>>         * tree-ssa-phiopt.c (spaceship_replacement): Likewise.
>>         * tree-ssa-reassoc.c (make_new_ssa_for_def): Likewise.
[...]
>> diff --git a/gcc/tree.c b/gcc/tree.c
>> index 7bfd64160f4..845228a055b 100644
>> --- a/gcc/tree.c
>> +++ b/gcc/tree.c
>> @@ -5280,6 +5280,18 @@ build_decl (location_t loc, enum tree_code code, tree name,
>>    return t;
>>  }
>>
>> +/* Create and return a DEBUG_EXPR_DECL node of the given TYPE.  */
>> +
>> +tree
>> +build_debug_expr_decl (tree type)
> Hi, sorry to nitpick, but would it be slightly better to use
> const_tree type instead ?
>
> Thanks,
> Prathamesh
>> +{
>> +  tree vexpr = make_node (DEBUG_EXPR_DECL);
>> +  DECL_ARTIFICIAL (vexpr) = 1;
>> +  TREE_TYPE (vexpr) = type;

I like the idea but the assignment to TREE_TYPE above could not be done
without discarding the const qualifier somehow.  Do we really want to do
that?  Do we have some canonical way of doing that?  (I searched only
for a minute but did not find any.)

Thanks,

Martin

Re: [PATCH] Introduce build_debug_expr_decl

[Richard Biener]

On Tue, 9 Nov 2021, Martin Jambor wrote:

> Hi,
> 
> On Mon, Nov 08 2021, Prathamesh Kulkarni wrote:
> > On Mon, 8 Nov 2021 at 23:24, Martin Jambor <mjambor@suse.cz> wrote:
> >>
> >> Hi,
> >>
> >> 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?
> >>
> >> Thanks,
> >>
> >> Martin
> >>
> >>
> >> gcc/ChangeLog:
> >>
> >> 2021-11-08  Martin Jambor  <mjambor@suse.cz>
> >>
> >>         * tree.h (build_debug_expr_decl): Declare.
> >>         * tree.c (build_debug_expr_decl): New function.
> >>         * cfgexpand.c (avoid_deep_ter_for_debug): Use build_debug_expr_decl
> >>         instead of building a DEBUG_EXPR_DECL.
> >>         * ipa-param-manipulation.c
> >>         (ipa_param_body_adjustments::prepare_debug_expressions): Likewise.
> >>         * omp-simd-clone.c (ipa_simd_modify_stmt_ops): Likewise.
> >>         * tree-ssa-ccp.c (optimize_atomic_bit_test_and): Likewise.
> >>         * tree-ssa-phiopt.c (spaceship_replacement): Likewise.
> >>         * tree-ssa-reassoc.c (make_new_ssa_for_def): Likewise.
> [...]
> >> diff --git a/gcc/tree.c b/gcc/tree.c
> >> index 7bfd64160f4..845228a055b 100644
> >> --- a/gcc/tree.c
> >> +++ b/gcc/tree.c
> >> @@ -5280,6 +5280,18 @@ build_decl (location_t loc, enum tree_code code, tree name,
> >>    return t;
> >>  }
> >>
> >> +/* Create and return a DEBUG_EXPR_DECL node of the given TYPE.  */
> >> +
> >> +tree
> >> +build_debug_expr_decl (tree type)
> > Hi, sorry to nitpick, but would it be slightly better to use
> > const_tree type instead ?
> >
> > Thanks,
> > Prathamesh
> >> +{
> >> +  tree vexpr = make_node (DEBUG_EXPR_DECL);
> >> +  DECL_ARTIFICIAL (vexpr) = 1;
> >> +  TREE_TYPE (vexpr) = type;
> 
> I like the idea but the assignment to TREE_TYPE above could not be done
> without discarding the const qualifier somehow.  Do we really want to do
> that?  Do we have some canonical way of doing that?  (I searched only
> for a minute but did not find any.)

Probably not worth the trouble then (adding some form of const_cast).

Richard.

[PATCH] Fortran: Use build_debug_expr_decl to create DEBUG_DECL_EXPRs

[Martin Jambor]

Hi,

This patch converts one more open coded construction of a
DEBUG_EXPR_DECL to a call of build_debug_expr_decl that I missed in my
previous patch because it happens to be in the Fortran front-end.

Bootstrapped and tested on x86_64-linux.  Since this should have
been done by an earlier approved patch, I consider it also approved
and will commit it in a moment.

Thanks,

Martin


gcc/fortran/ChangeLog:

2021-11-11  Martin Jambor  <mjambor@suse.cz>

	* trans-types.c (gfc_get_array_descr_info): Use build_debug_expr_decl
	instead of building DEBUG_EXPR_DECL manually.
---
 gcc/fortran/trans-types.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/gcc/fortran/trans-types.c b/gcc/fortran/trans-types.c
index 947ab5a099b..e5d36d5a58f 100644
--- a/gcc/fortran/trans-types.c
+++ b/gcc/fortran/trans-types.c
@@ -3417,10 +3417,8 @@ gfc_get_array_descr_info (const_tree type, struct array_descr_info *info)
   base_decl = GFC_TYPE_ARRAY_BASE_DECL (type, indirect);
   if (!base_decl)
     {
-      base_decl = make_node (DEBUG_EXPR_DECL);
-      DECL_ARTIFICIAL (base_decl) = 1;
-      TREE_TYPE (base_decl) = indirect ? build_pointer_type (ptype) : ptype;
-      SET_DECL_MODE (base_decl, TYPE_MODE (TREE_TYPE (base_decl)));
+      base_decl = build_debug_expr_decl (indirect
+					 ? build_pointer_type (ptype) : ptype);
       GFC_TYPE_ARRAY_BASE_DECL (type, indirect) = base_decl;
     }
   info->base_decl = base_decl;
-- 
2.33.0

[PATCH] Replace more DEBUG_EXPR_DECL creations with build_debug_expr_decl

[Martin Jambor]

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  <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
-- 
2.33.0

Re: [PATCH] Replace more DEBUG_EXPR_DECL creations with build_debug_expr_decl

[Richard Biener]

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