public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
* [PATCH 1/3] Fix printing COMPOUND_EXPR in .original [PR23872]
@ 2024-05-02 21:39 Andrew Pinski
  2024-05-02 21:39 ` [PATCH 2/3] Improve DECL_EXPR printing " Andrew Pinski
                   ` (2 more replies)
  0 siblings, 3 replies; 8+ messages in thread
From: Andrew Pinski @ 2024-05-02 21:39 UTC (permalink / raw)
  To: gcc-patches; +Cc: Andrew Pinski

Starting with the merge of the openmp branch into the trunk
(r0-73077-g953ff28998b59b), COMPOUND_EXPR started to be printed
as `expr; , expr` which is wrong. This was due to the wrong
conversion of dumping_stmts into `!(flags & TDF_SLIM)`. That is wrong
as we are not dumping stmts at this point (`!(flags & TDF_SLIM)` was always
true for this case as TDF_SLIM case was handled before hand). So switch it
to be always false.

Bootstrapped and tested on x86_64-linux-gnu with no regressions.

gcc/ChangeLog:

	PR middle-end/23872
	* tree-pretty-print.cc (dump_generic_node <case COMPOUND_EXPR>): Fix
	calls to dump_generic_node and also remove unreachable code that is testing
	`flags & TDF_SLIM`.

gcc/testsuite/ChangeLog:

	* gfortran.dg/gomp/atomic-21.f90: Update testcase for the removal of `;`.

Signed-off-by: Andrew Pinski <quic_apinski@quicinc.com>
---
 gcc/testsuite/gfortran.dg/gomp/atomic-21.f90 |  4 ++--
 gcc/tree-pretty-print.cc                     | 24 ++++++--------------
 2 files changed, 9 insertions(+), 19 deletions(-)

diff --git a/gcc/testsuite/gfortran.dg/gomp/atomic-21.f90 b/gcc/testsuite/gfortran.dg/gomp/atomic-21.f90
index febcdbbacfb..35099294d7a 100644
--- a/gcc/testsuite/gfortran.dg/gomp/atomic-21.f90
+++ b/gcc/testsuite/gfortran.dg/gomp/atomic-21.f90
@@ -56,7 +56,7 @@ subroutine foobar()
     endif
 
 !  TARGET_EXPR <D.4244, 0> = #pragma omp atomic capture acq_rel
-!    TARGET_EXPR <D.4242, &nn> = NON_LVALUE_EXPR <TARGET_EXPR <D.4243, 0> = *TARGET_EXPR <D.4242, &nn> == oo> ? pp : *TARGET_EXPR <D.4242, &nn>;, if (TARGET_EXPR <D.4243, 0>)
+!    TARGET_EXPR <D.4242, &nn> = NON_LVALUE_EXPR <TARGET_EXPR <D.4243, 0> = *TARGET_EXPR <D.4242, &nn> == oo> ? pp : *TARGET_EXPR <D.4242, &nn>, if (TARGET_EXPR <D.4243, 0>)
 !    {
 !      <<< Unknown tree: void_cst >>>
 !    }
@@ -66,7 +66,7 @@ subroutine foobar()
 !    };
 !
 ! { dg-final { scan-tree-dump-times "TARGET_EXPR <D.\[0-9\]+, 0> = #pragma omp atomic capture acq_rel" 1 "original" } }
-! { dg-final { scan-tree-dump-times "TARGET_EXPR <D.\[0-9\]+, &nn> = NON_LVALUE_EXPR <TARGET_EXPR <D.\[0-9\]+, 0> = \\*TARGET_EXPR <D.\[0-9\]+, &nn> == oo> \\? pp : \\*TARGET_EXPR <D.\[0-9\]+, &nn>;, if \\(TARGET_EXPR <D.\[0-9\]+, 0>\\)" 1 "original" } }
+! { dg-final { scan-tree-dump-times "TARGET_EXPR <D.\[0-9\]+, &nn> = NON_LVALUE_EXPR <TARGET_EXPR <D.\[0-9\]+, 0> = \\*TARGET_EXPR <D.\[0-9\]+, &nn> == oo> \\? pp : \\*TARGET_EXPR <D.\[0-9\]+, &nn>, if \\(TARGET_EXPR <D.\[0-9\]+, 0>\\)" 1 "original" } }
 ! { dg-final { scan-tree-dump-times "<<< Unknown tree: void_cst >>>" 1 "original" } }
 ! { dg-final { scan-tree-dump-times "qq = TARGET_EXPR <D.\[0-9\]+, 0>;" 1 "original" } }
 
diff --git a/gcc/tree-pretty-print.cc b/gcc/tree-pretty-print.cc
index c935a7da7d1..f9ad8562078 100644
--- a/gcc/tree-pretty-print.cc
+++ b/gcc/tree-pretty-print.cc
@@ -2856,31 +2856,21 @@ dump_generic_node (pretty_printer *pp, tree node, int spc, dump_flags_t flags,
 	  }
 
 	dump_generic_node (pp, TREE_OPERAND (node, 0),
-			   spc, flags, !(flags & TDF_SLIM));
-	if (flags & TDF_SLIM)
-	  newline_and_indent (pp, spc);
-	else
-	  {
-	    pp_comma (pp);
-	    pp_space (pp);
-	  }
+			   spc, flags, false);
+	pp_comma (pp);
+	pp_space (pp);
 
 	for (tp = &TREE_OPERAND (node, 1);
 	     TREE_CODE (*tp) == COMPOUND_EXPR;
 	     tp = &TREE_OPERAND (*tp, 1))
 	  {
 	    dump_generic_node (pp, TREE_OPERAND (*tp, 0),
-			       spc, flags, !(flags & TDF_SLIM));
-	    if (flags & TDF_SLIM)
-	      newline_and_indent (pp, spc);
-	    else
-	      {
-	        pp_comma (pp);
-	        pp_space (pp);
-	      }
+			       spc, flags, false);
+	    pp_comma (pp);
+	    pp_space (pp);
 	  }
 
-	dump_generic_node (pp, *tp, spc, flags, !(flags & TDF_SLIM));
+	dump_generic_node (pp, *tp, spc, flags, false);
       }
       break;
 
-- 
2.43.0


^ permalink raw reply	[flat|nested] 8+ messages in thread

* [PATCH 2/3] Improve DECL_EXPR printing in .original [PR23872]
  2024-05-02 21:39 [PATCH 1/3] Fix printing COMPOUND_EXPR in .original [PR23872] Andrew Pinski
@ 2024-05-02 21:39 ` Andrew Pinski
  2024-05-03 11:34   ` Richard Biener
  2024-05-02 21:39 ` [PATCH 3/3] Add parentheses around DECL_INIT for " Andrew Pinski
  2024-05-03 11:29 ` [PATCH 1/3] Fix printing COMPOUND_EXPR in " Richard Biener
  2 siblings, 1 reply; 8+ messages in thread
From: Andrew Pinski @ 2024-05-02 21:39 UTC (permalink / raw)
  To: gcc-patches; +Cc: Andrew Pinski

Right now we don't print that a DECL_EXPR and we get
basically double output of the decls and it looks confusing.
This fixes that.
for the simple example:
`void foo () { int result = 0;}`
This gives:
```
{
  int result = 0;

  DECL_EXPR<int result>;
}
```

Bootstrapped and tested on x86_64-linux-gnu with no regressions.

gcc/ChangeLog:

	* tree-pretty-print.cc (dump_generic_node <case DECL_EXPR>): Print
	out `DECL_EXPR<...>` around the decl and update the call to
	print_declaration to pass false for new argument and pass 0
	for the spacing.
	(print_declaration): Add argument is_stmt and don't print
	a semicolon nor the initializer.
	* tree-pretty-print.h (print_declaration): Add bool argument
	and default it to true.

Signed-off-by: Andrew Pinski <quic_apinski@quicinc.com>
---
 gcc/tree-pretty-print.cc | 18 +++++++++++-------
 gcc/tree-pretty-print.h  |  2 +-
 2 files changed, 12 insertions(+), 8 deletions(-)

diff --git a/gcc/tree-pretty-print.cc b/gcc/tree-pretty-print.cc
index f9ad8562078..825ba74443b 100644
--- a/gcc/tree-pretty-print.cc
+++ b/gcc/tree-pretty-print.cc
@@ -2917,8 +2917,9 @@ dump_generic_node (pretty_printer *pp, tree node, int spc, dump_flags_t flags,
       break;
 
     case DECL_EXPR:
-      print_declaration (pp, DECL_EXPR_DECL (node), spc, flags);
-      is_stmt = false;
+      pp_string (pp, "DECL_EXPR<");
+      print_declaration (pp, DECL_EXPR_DECL (node), 0, flags, false);
+      pp_greater (pp);
       break;
 
     case COND_EXPR:
@@ -4151,10 +4152,11 @@ dump_generic_node (pretty_printer *pp, tree node, int spc, dump_flags_t flags,
   return spc;
 }
 
-/* Print the declaration of a variable.  */
+/* Print the declaration of a variable, T to PP starting with SPC spaces with FLAGS
+   and called as IS_STMT a statement or not.  */
 
 void
-print_declaration (pretty_printer *pp, tree t, int spc, dump_flags_t flags)
+print_declaration (pretty_printer *pp, tree t, int spc, dump_flags_t flags, bool is_stmt)
 {
   INDENT (spc);
 
@@ -4162,7 +4164,8 @@ print_declaration (pretty_printer *pp, tree t, int spc, dump_flags_t flags)
     {
       pp_string(pp, "namelist ");
       dump_decl_name (pp, t, flags);
-      pp_semicolon (pp);
+      if (is_stmt)
+	pp_semicolon (pp);
       return;
     }
 
@@ -4231,7 +4234,7 @@ print_declaration (pretty_printer *pp, tree t, int spc, dump_flags_t flags)
   if (TREE_CODE (t) != FUNCTION_DECL)
     {
       /* Print the initial value.  */
-      if (DECL_INITIAL (t))
+      if (DECL_INITIAL (t) && is_stmt)
 	{
 	  pp_space (pp);
 	  pp_equal (pp);
@@ -4250,7 +4253,8 @@ print_declaration (pretty_printer *pp, tree t, int spc, dump_flags_t flags)
       pp_right_bracket (pp);
     }
 
-  pp_semicolon (pp);
+  if (is_stmt)
+    pp_semicolon (pp);
 }
 
 
diff --git a/gcc/tree-pretty-print.h b/gcc/tree-pretty-print.h
index 0da6242629b..660c17410a9 100644
--- a/gcc/tree-pretty-print.h
+++ b/gcc/tree-pretty-print.h
@@ -47,7 +47,7 @@ extern void dump_omp_loop_non_rect_expr (pretty_printer *, tree, int,
 					 dump_flags_t);
 extern void print_omp_context_selector (FILE *, tree, dump_flags_t);
 extern int dump_generic_node (pretty_printer *, tree, int, dump_flags_t, bool);
-extern void print_declaration (pretty_printer *, tree, int, dump_flags_t);
+extern void print_declaration (pretty_printer *, tree, int, dump_flags_t, bool = true);
 extern int op_code_prio (enum tree_code);
 extern int op_prio (const_tree);
 extern const char *op_symbol_code (enum tree_code, dump_flags_t = TDF_NONE);
-- 
2.43.0


^ permalink raw reply	[flat|nested] 8+ messages in thread

* [PATCH 3/3] Add parentheses around DECL_INIT for .original [PR23872]
  2024-05-02 21:39 [PATCH 1/3] Fix printing COMPOUND_EXPR in .original [PR23872] Andrew Pinski
  2024-05-02 21:39 ` [PATCH 2/3] Improve DECL_EXPR printing " Andrew Pinski
@ 2024-05-02 21:39 ` Andrew Pinski
  2024-05-03 11:40   ` Richard Biener
  2024-05-03 11:29 ` [PATCH 1/3] Fix printing COMPOUND_EXPR in " Richard Biener
  2 siblings, 1 reply; 8+ messages in thread
From: Andrew Pinski @ 2024-05-02 21:39 UTC (permalink / raw)
  To: gcc-patches; +Cc: Andrew Pinski

When we have :
`void f (int y, int z) { int x = ( z++,y); }`

This would have printed the decl's initializer without
parentheses which can confusion if you think that is defining
another variable rather than the compound expression.

This adds parenthese around DECL_INIT if it was a COMPOUND_EXPR.

Bootstrapped and tested on x86_64-linux-gnu.

gcc/ChangeLog:

	* tree-pretty-print.cc (print_declaration): Add parenthese
	around DECL_INIT if it was a COMPOUND_EXPR.

Signed-off-by: Andrew Pinski <quic_apinski@quicinc.com>
---
 gcc/tree-pretty-print.cc | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/gcc/tree-pretty-print.cc b/gcc/tree-pretty-print.cc
index 825ba74443b..8b766dcd2b8 100644
--- a/gcc/tree-pretty-print.cc
+++ b/gcc/tree-pretty-print.cc
@@ -4240,7 +4240,14 @@ print_declaration (pretty_printer *pp, tree t, int spc, dump_flags_t flags, bool
 	  pp_equal (pp);
 	  pp_space (pp);
 	  if (!(flags & TDF_SLIM))
-	    dump_generic_node (pp, DECL_INITIAL (t), spc, flags, false);
+	    {
+	      bool need_paren = TREE_CODE (DECL_INITIAL (t)) == COMPOUND_EXPR;
+	      if (need_paren)
+		pp_left_paren (pp);
+	      dump_generic_node (pp, DECL_INITIAL (t), spc, flags, false);
+	      if (need_paren)
+		pp_right_paren (pp);
+	    }
 	  else
 	    pp_string (pp, "<<< omitted >>>");
 	}
-- 
2.43.0


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH 1/3] Fix printing COMPOUND_EXPR in .original [PR23872]
  2024-05-02 21:39 [PATCH 1/3] Fix printing COMPOUND_EXPR in .original [PR23872] Andrew Pinski
  2024-05-02 21:39 ` [PATCH 2/3] Improve DECL_EXPR printing " Andrew Pinski
  2024-05-02 21:39 ` [PATCH 3/3] Add parentheses around DECL_INIT for " Andrew Pinski
@ 2024-05-03 11:29 ` Richard Biener
  2 siblings, 0 replies; 8+ messages in thread
From: Richard Biener @ 2024-05-03 11:29 UTC (permalink / raw)
  To: Andrew Pinski; +Cc: gcc-patches

On Thu, May 2, 2024 at 11:40 PM Andrew Pinski <quic_apinski@quicinc.com> wrote:
>
> Starting with the merge of the openmp branch into the trunk
> (r0-73077-g953ff28998b59b), COMPOUND_EXPR started to be printed
> as `expr; , expr` which is wrong. This was due to the wrong
> conversion of dumping_stmts into `!(flags & TDF_SLIM)`. That is wrong
> as we are not dumping stmts at this point (`!(flags & TDF_SLIM)` was always
> true for this case as TDF_SLIM case was handled before hand). So switch it
> to be always false.
>
> Bootstrapped and tested on x86_64-linux-gnu with no regressions.

OK.

> gcc/ChangeLog:
>
>         PR middle-end/23872
>         * tree-pretty-print.cc (dump_generic_node <case COMPOUND_EXPR>): Fix
>         calls to dump_generic_node and also remove unreachable code that is testing
>         `flags & TDF_SLIM`.
>
> gcc/testsuite/ChangeLog:
>
>         * gfortran.dg/gomp/atomic-21.f90: Update testcase for the removal of `;`.
>
> Signed-off-by: Andrew Pinski <quic_apinski@quicinc.com>
> ---
>  gcc/testsuite/gfortran.dg/gomp/atomic-21.f90 |  4 ++--
>  gcc/tree-pretty-print.cc                     | 24 ++++++--------------
>  2 files changed, 9 insertions(+), 19 deletions(-)
>
> diff --git a/gcc/testsuite/gfortran.dg/gomp/atomic-21.f90 b/gcc/testsuite/gfortran.dg/gomp/atomic-21.f90
> index febcdbbacfb..35099294d7a 100644
> --- a/gcc/testsuite/gfortran.dg/gomp/atomic-21.f90
> +++ b/gcc/testsuite/gfortran.dg/gomp/atomic-21.f90
> @@ -56,7 +56,7 @@ subroutine foobar()
>      endif
>
>  !  TARGET_EXPR <D.4244, 0> = #pragma omp atomic capture acq_rel
> -!    TARGET_EXPR <D.4242, &nn> = NON_LVALUE_EXPR <TARGET_EXPR <D.4243, 0> = *TARGET_EXPR <D.4242, &nn> == oo> ? pp : *TARGET_EXPR <D.4242, &nn>;, if (TARGET_EXPR <D.4243, 0>)
> +!    TARGET_EXPR <D.4242, &nn> = NON_LVALUE_EXPR <TARGET_EXPR <D.4243, 0> = *TARGET_EXPR <D.4242, &nn> == oo> ? pp : *TARGET_EXPR <D.4242, &nn>, if (TARGET_EXPR <D.4243, 0>)
>  !    {
>  !      <<< Unknown tree: void_cst >>>
>  !    }
> @@ -66,7 +66,7 @@ subroutine foobar()
>  !    };
>  !
>  ! { dg-final { scan-tree-dump-times "TARGET_EXPR <D.\[0-9\]+, 0> = #pragma omp atomic capture acq_rel" 1 "original" } }
> -! { dg-final { scan-tree-dump-times "TARGET_EXPR <D.\[0-9\]+, &nn> = NON_LVALUE_EXPR <TARGET_EXPR <D.\[0-9\]+, 0> = \\*TARGET_EXPR <D.\[0-9\]+, &nn> == oo> \\? pp : \\*TARGET_EXPR <D.\[0-9\]+, &nn>;, if \\(TARGET_EXPR <D.\[0-9\]+, 0>\\)" 1 "original" } }
> +! { dg-final { scan-tree-dump-times "TARGET_EXPR <D.\[0-9\]+, &nn> = NON_LVALUE_EXPR <TARGET_EXPR <D.\[0-9\]+, 0> = \\*TARGET_EXPR <D.\[0-9\]+, &nn> == oo> \\? pp : \\*TARGET_EXPR <D.\[0-9\]+, &nn>, if \\(TARGET_EXPR <D.\[0-9\]+, 0>\\)" 1 "original" } }
>  ! { dg-final { scan-tree-dump-times "<<< Unknown tree: void_cst >>>" 1 "original" } }
>  ! { dg-final { scan-tree-dump-times "qq = TARGET_EXPR <D.\[0-9\]+, 0>;" 1 "original" } }
>
> diff --git a/gcc/tree-pretty-print.cc b/gcc/tree-pretty-print.cc
> index c935a7da7d1..f9ad8562078 100644
> --- a/gcc/tree-pretty-print.cc
> +++ b/gcc/tree-pretty-print.cc
> @@ -2856,31 +2856,21 @@ dump_generic_node (pretty_printer *pp, tree node, int spc, dump_flags_t flags,
>           }
>
>         dump_generic_node (pp, TREE_OPERAND (node, 0),
> -                          spc, flags, !(flags & TDF_SLIM));
> -       if (flags & TDF_SLIM)
> -         newline_and_indent (pp, spc);
> -       else
> -         {
> -           pp_comma (pp);
> -           pp_space (pp);
> -         }
> +                          spc, flags, false);
> +       pp_comma (pp);
> +       pp_space (pp);
>
>         for (tp = &TREE_OPERAND (node, 1);
>              TREE_CODE (*tp) == COMPOUND_EXPR;
>              tp = &TREE_OPERAND (*tp, 1))
>           {
>             dump_generic_node (pp, TREE_OPERAND (*tp, 0),
> -                              spc, flags, !(flags & TDF_SLIM));
> -           if (flags & TDF_SLIM)
> -             newline_and_indent (pp, spc);
> -           else
> -             {
> -               pp_comma (pp);
> -               pp_space (pp);
> -             }
> +                              spc, flags, false);
> +           pp_comma (pp);
> +           pp_space (pp);
>           }
>
> -       dump_generic_node (pp, *tp, spc, flags, !(flags & TDF_SLIM));
> +       dump_generic_node (pp, *tp, spc, flags, false);
>        }
>        break;
>
> --
> 2.43.0
>

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH 2/3] Improve DECL_EXPR printing in .original [PR23872]
  2024-05-02 21:39 ` [PATCH 2/3] Improve DECL_EXPR printing " Andrew Pinski
@ 2024-05-03 11:34   ` Richard Biener
  2024-05-03 22:11     ` Andrew Pinski
  0 siblings, 1 reply; 8+ messages in thread
From: Richard Biener @ 2024-05-03 11:34 UTC (permalink / raw)
  To: Andrew Pinski; +Cc: gcc-patches

On Thu, May 2, 2024 at 11:40 PM Andrew Pinski <quic_apinski@quicinc.com> wrote:
>
> Right now we don't print that a DECL_EXPR and we get
> basically double output of the decls and it looks confusing.
> This fixes that.
> for the simple example:
> `void foo () { int result = 0;}`
> This gives:
> ```
> {
>   int result = 0;
>
>   DECL_EXPR<int result>;
> }
> ```

Hmm, I think it would be better if it were

  {

    int result = 0;
  }

so omit the dumping from the BLOCK_VARS(?) when the variable has a DECL_EXPR.
That more easily lets us spot use-before-DECL_EXPR issues.

So I don't think this patch is an improvement?

Thanks,
Richard.

> Bootstrapped and tested on x86_64-linux-gnu with no regressions.
>
> gcc/ChangeLog:
>
>         * tree-pretty-print.cc (dump_generic_node <case DECL_EXPR>): Print
>         out `DECL_EXPR<...>` around the decl and update the call to
>         print_declaration to pass false for new argument and pass 0
>         for the spacing.
>         (print_declaration): Add argument is_stmt and don't print
>         a semicolon nor the initializer.
>         * tree-pretty-print.h (print_declaration): Add bool argument
>         and default it to true.
>
> Signed-off-by: Andrew Pinski <quic_apinski@quicinc.com>
> ---
>  gcc/tree-pretty-print.cc | 18 +++++++++++-------
>  gcc/tree-pretty-print.h  |  2 +-
>  2 files changed, 12 insertions(+), 8 deletions(-)
>
> diff --git a/gcc/tree-pretty-print.cc b/gcc/tree-pretty-print.cc
> index f9ad8562078..825ba74443b 100644
> --- a/gcc/tree-pretty-print.cc
> +++ b/gcc/tree-pretty-print.cc
> @@ -2917,8 +2917,9 @@ dump_generic_node (pretty_printer *pp, tree node, int spc, dump_flags_t flags,
>        break;
>
>      case DECL_EXPR:
> -      print_declaration (pp, DECL_EXPR_DECL (node), spc, flags);
> -      is_stmt = false;
> +      pp_string (pp, "DECL_EXPR<");
> +      print_declaration (pp, DECL_EXPR_DECL (node), 0, flags, false);
> +      pp_greater (pp);
>        break;
>
>      case COND_EXPR:
> @@ -4151,10 +4152,11 @@ dump_generic_node (pretty_printer *pp, tree node, int spc, dump_flags_t flags,
>    return spc;
>  }
>
> -/* Print the declaration of a variable.  */
> +/* Print the declaration of a variable, T to PP starting with SPC spaces with FLAGS
> +   and called as IS_STMT a statement or not.  */
>
>  void
> -print_declaration (pretty_printer *pp, tree t, int spc, dump_flags_t flags)
> +print_declaration (pretty_printer *pp, tree t, int spc, dump_flags_t flags, bool is_stmt)
>  {
>    INDENT (spc);
>
> @@ -4162,7 +4164,8 @@ print_declaration (pretty_printer *pp, tree t, int spc, dump_flags_t flags)
>      {
>        pp_string(pp, "namelist ");
>        dump_decl_name (pp, t, flags);
> -      pp_semicolon (pp);
> +      if (is_stmt)
> +       pp_semicolon (pp);
>        return;
>      }
>
> @@ -4231,7 +4234,7 @@ print_declaration (pretty_printer *pp, tree t, int spc, dump_flags_t flags)
>    if (TREE_CODE (t) != FUNCTION_DECL)
>      {
>        /* Print the initial value.  */
> -      if (DECL_INITIAL (t))
> +      if (DECL_INITIAL (t) && is_stmt)
>         {
>           pp_space (pp);
>           pp_equal (pp);
> @@ -4250,7 +4253,8 @@ print_declaration (pretty_printer *pp, tree t, int spc, dump_flags_t flags)
>        pp_right_bracket (pp);
>      }
>
> -  pp_semicolon (pp);
> +  if (is_stmt)
> +    pp_semicolon (pp);
>  }
>
>
> diff --git a/gcc/tree-pretty-print.h b/gcc/tree-pretty-print.h
> index 0da6242629b..660c17410a9 100644
> --- a/gcc/tree-pretty-print.h
> +++ b/gcc/tree-pretty-print.h
> @@ -47,7 +47,7 @@ extern void dump_omp_loop_non_rect_expr (pretty_printer *, tree, int,
>                                          dump_flags_t);
>  extern void print_omp_context_selector (FILE *, tree, dump_flags_t);
>  extern int dump_generic_node (pretty_printer *, tree, int, dump_flags_t, bool);
> -extern void print_declaration (pretty_printer *, tree, int, dump_flags_t);
> +extern void print_declaration (pretty_printer *, tree, int, dump_flags_t, bool = true);
>  extern int op_code_prio (enum tree_code);
>  extern int op_prio (const_tree);
>  extern const char *op_symbol_code (enum tree_code, dump_flags_t = TDF_NONE);
> --
> 2.43.0
>

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH 3/3] Add parentheses around DECL_INIT for .original [PR23872]
  2024-05-02 21:39 ` [PATCH 3/3] Add parentheses around DECL_INIT for " Andrew Pinski
@ 2024-05-03 11:40   ` Richard Biener
  2024-05-03 22:12     ` Andrew Pinski
  0 siblings, 1 reply; 8+ messages in thread
From: Richard Biener @ 2024-05-03 11:40 UTC (permalink / raw)
  To: Andrew Pinski; +Cc: gcc-patches

On Thu, May 2, 2024 at 11:40 PM Andrew Pinski <quic_apinski@quicinc.com> wrote:
>
> When we have :
> `void f (int y, int z) { int x = ( z++,y); }`
>
> This would have printed the decl's initializer without
> parentheses which can confusion if you think that is defining
> another variable rather than the compound expression.
>
> This adds parenthese around DECL_INIT if it was a COMPOUND_EXPR.

Looking it seems we'd hit a similar issue for

 foo ((z++,y), 2);

thus in CALL_EXPR context.  Also

int k;
void foo (int i, int j)
{
  k = (i, 2) + j;
}

dumps as

{
  k = i, j + 2;;
}

(ok that's folded to (i, j + 2) but still).

So shouldn't we bite the bullet and wrap all COMPOUND_EXPRs in
parens instead?  Possibly "tail-calling" the case of
a, b, c in COMPOUND_EXPR dumping itself?

Thanks,
Richard.

> Bootstrapped and tested on x86_64-linux-gnu.
>
> gcc/ChangeLog:
>
>         * tree-pretty-print.cc (print_declaration): Add parenthese
>         around DECL_INIT if it was a COMPOUND_EXPR.
>
> Signed-off-by: Andrew Pinski <quic_apinski@quicinc.com>
> ---
>  gcc/tree-pretty-print.cc | 9 ++++++++-
>  1 file changed, 8 insertions(+), 1 deletion(-)
>
> diff --git a/gcc/tree-pretty-print.cc b/gcc/tree-pretty-print.cc
> index 825ba74443b..8b766dcd2b8 100644
> --- a/gcc/tree-pretty-print.cc
> +++ b/gcc/tree-pretty-print.cc
> @@ -4240,7 +4240,14 @@ print_declaration (pretty_printer *pp, tree t, int spc, dump_flags_t flags, bool
>           pp_equal (pp);
>           pp_space (pp);
>           if (!(flags & TDF_SLIM))
> -           dump_generic_node (pp, DECL_INITIAL (t), spc, flags, false);
> +           {
> +             bool need_paren = TREE_CODE (DECL_INITIAL (t)) == COMPOUND_EXPR;
> +             if (need_paren)
> +               pp_left_paren (pp);
> +             dump_generic_node (pp, DECL_INITIAL (t), spc, flags, false);
> +             if (need_paren)
> +               pp_right_paren (pp);
> +           }
>           else
>             pp_string (pp, "<<< omitted >>>");
>         }
> --
> 2.43.0
>

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH 2/3] Improve DECL_EXPR printing in .original [PR23872]
  2024-05-03 11:34   ` Richard Biener
@ 2024-05-03 22:11     ` Andrew Pinski
  0 siblings, 0 replies; 8+ messages in thread
From: Andrew Pinski @ 2024-05-03 22:11 UTC (permalink / raw)
  To: Richard Biener; +Cc: Andrew Pinski, gcc-patches

On Fri, May 3, 2024 at 4:36 AM Richard Biener
<richard.guenther@gmail.com> wrote:
>
> On Thu, May 2, 2024 at 11:40 PM Andrew Pinski <quic_apinski@quicinc.com> wrote:
> >
> > Right now we don't print that a DECL_EXPR and we get
> > basically double output of the decls and it looks confusing.
> > This fixes that.
> > for the simple example:
> > `void foo () { int result = 0;}`
> > This gives:
> > ```
> > {
> >   int result = 0;
> >
> >   DECL_EXPR<int result>;
> > }
> > ```
>
> Hmm, I think it would be better if it were
>
>   {
>
>     int result = 0;
>   }
>
> so omit the dumping from the BLOCK_VARS(?) when the variable has a DECL_EXPR.
> That more easily lets us spot use-before-DECL_EXPR issues.

Yes, yes that would be definitely better. Let me see if I can figure
that out. I might not be able to get back to this until June though.

>
> So I don't think this patch is an improvement?

Ok, yes I agree.

Thanks,
Andrew

>
> Thanks,
> Richard.
>
> > Bootstrapped and tested on x86_64-linux-gnu with no regressions.
> >
> > gcc/ChangeLog:
> >
> >         * tree-pretty-print.cc (dump_generic_node <case DECL_EXPR>): Print
> >         out `DECL_EXPR<...>` around the decl and update the call to
> >         print_declaration to pass false for new argument and pass 0
> >         for the spacing.
> >         (print_declaration): Add argument is_stmt and don't print
> >         a semicolon nor the initializer.
> >         * tree-pretty-print.h (print_declaration): Add bool argument
> >         and default it to true.
> >
> > Signed-off-by: Andrew Pinski <quic_apinski@quicinc.com>
> > ---
> >  gcc/tree-pretty-print.cc | 18 +++++++++++-------
> >  gcc/tree-pretty-print.h  |  2 +-
> >  2 files changed, 12 insertions(+), 8 deletions(-)
> >
> > diff --git a/gcc/tree-pretty-print.cc b/gcc/tree-pretty-print.cc
> > index f9ad8562078..825ba74443b 100644
> > --- a/gcc/tree-pretty-print.cc
> > +++ b/gcc/tree-pretty-print.cc
> > @@ -2917,8 +2917,9 @@ dump_generic_node (pretty_printer *pp, tree node, int spc, dump_flags_t flags,
> >        break;
> >
> >      case DECL_EXPR:
> > -      print_declaration (pp, DECL_EXPR_DECL (node), spc, flags);
> > -      is_stmt = false;
> > +      pp_string (pp, "DECL_EXPR<");
> > +      print_declaration (pp, DECL_EXPR_DECL (node), 0, flags, false);
> > +      pp_greater (pp);
> >        break;
> >
> >      case COND_EXPR:
> > @@ -4151,10 +4152,11 @@ dump_generic_node (pretty_printer *pp, tree node, int spc, dump_flags_t flags,
> >    return spc;
> >  }
> >
> > -/* Print the declaration of a variable.  */
> > +/* Print the declaration of a variable, T to PP starting with SPC spaces with FLAGS
> > +   and called as IS_STMT a statement or not.  */
> >
> >  void
> > -print_declaration (pretty_printer *pp, tree t, int spc, dump_flags_t flags)
> > +print_declaration (pretty_printer *pp, tree t, int spc, dump_flags_t flags, bool is_stmt)
> >  {
> >    INDENT (spc);
> >
> > @@ -4162,7 +4164,8 @@ print_declaration (pretty_printer *pp, tree t, int spc, dump_flags_t flags)
> >      {
> >        pp_string(pp, "namelist ");
> >        dump_decl_name (pp, t, flags);
> > -      pp_semicolon (pp);
> > +      if (is_stmt)
> > +       pp_semicolon (pp);
> >        return;
> >      }
> >
> > @@ -4231,7 +4234,7 @@ print_declaration (pretty_printer *pp, tree t, int spc, dump_flags_t flags)
> >    if (TREE_CODE (t) != FUNCTION_DECL)
> >      {
> >        /* Print the initial value.  */
> > -      if (DECL_INITIAL (t))
> > +      if (DECL_INITIAL (t) && is_stmt)
> >         {
> >           pp_space (pp);
> >           pp_equal (pp);
> > @@ -4250,7 +4253,8 @@ print_declaration (pretty_printer *pp, tree t, int spc, dump_flags_t flags)
> >        pp_right_bracket (pp);
> >      }
> >
> > -  pp_semicolon (pp);
> > +  if (is_stmt)
> > +    pp_semicolon (pp);
> >  }
> >
> >
> > diff --git a/gcc/tree-pretty-print.h b/gcc/tree-pretty-print.h
> > index 0da6242629b..660c17410a9 100644
> > --- a/gcc/tree-pretty-print.h
> > +++ b/gcc/tree-pretty-print.h
> > @@ -47,7 +47,7 @@ extern void dump_omp_loop_non_rect_expr (pretty_printer *, tree, int,
> >                                          dump_flags_t);
> >  extern void print_omp_context_selector (FILE *, tree, dump_flags_t);
> >  extern int dump_generic_node (pretty_printer *, tree, int, dump_flags_t, bool);
> > -extern void print_declaration (pretty_printer *, tree, int, dump_flags_t);
> > +extern void print_declaration (pretty_printer *, tree, int, dump_flags_t, bool = true);
> >  extern int op_code_prio (enum tree_code);
> >  extern int op_prio (const_tree);
> >  extern const char *op_symbol_code (enum tree_code, dump_flags_t = TDF_NONE);
> > --
> > 2.43.0
> >

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH 3/3] Add parentheses around DECL_INIT for .original [PR23872]
  2024-05-03 11:40   ` Richard Biener
@ 2024-05-03 22:12     ` Andrew Pinski
  0 siblings, 0 replies; 8+ messages in thread
From: Andrew Pinski @ 2024-05-03 22:12 UTC (permalink / raw)
  To: Richard Biener; +Cc: Andrew Pinski, gcc-patches

On Fri, May 3, 2024 at 4:41 AM Richard Biener
<richard.guenther@gmail.com> wrote:
>
> On Thu, May 2, 2024 at 11:40 PM Andrew Pinski <quic_apinski@quicinc.com> wrote:
> >
> > When we have :
> > `void f (int y, int z) { int x = ( z++,y); }`
> >
> > This would have printed the decl's initializer without
> > parentheses which can confusion if you think that is defining
> > another variable rather than the compound expression.
> >
> > This adds parenthese around DECL_INIT if it was a COMPOUND_EXPR.
>
> Looking it seems we'd hit a similar issue for
>
>  foo ((z++,y), 2);
>
> thus in CALL_EXPR context.  Also
>
> int k;
> void foo (int i, int j)
> {
>   k = (i, 2) + j;
> }
>
> dumps as
>
> {
>   k = i, j + 2;;
> }
>
> (ok that's folded to (i, j + 2) but still).
>
> So shouldn't we bite the bullet and wrap all COMPOUND_EXPRs in
> parens instead?  Possibly "tail-calling" the case of
> a, b, c in COMPOUND_EXPR dumping itself?

Let me look into that but it won't be until June due to other things going on.

Thanks,
Andrew

>
> Thanks,
> Richard.
>
> > Bootstrapped and tested on x86_64-linux-gnu.
> >
> > gcc/ChangeLog:
> >
> >         * tree-pretty-print.cc (print_declaration): Add parenthese
> >         around DECL_INIT if it was a COMPOUND_EXPR.
> >
> > Signed-off-by: Andrew Pinski <quic_apinski@quicinc.com>
> > ---
> >  gcc/tree-pretty-print.cc | 9 ++++++++-
> >  1 file changed, 8 insertions(+), 1 deletion(-)
> >
> > diff --git a/gcc/tree-pretty-print.cc b/gcc/tree-pretty-print.cc
> > index 825ba74443b..8b766dcd2b8 100644
> > --- a/gcc/tree-pretty-print.cc
> > +++ b/gcc/tree-pretty-print.cc
> > @@ -4240,7 +4240,14 @@ print_declaration (pretty_printer *pp, tree t, int spc, dump_flags_t flags, bool
> >           pp_equal (pp);
> >           pp_space (pp);
> >           if (!(flags & TDF_SLIM))
> > -           dump_generic_node (pp, DECL_INITIAL (t), spc, flags, false);
> > +           {
> > +             bool need_paren = TREE_CODE (DECL_INITIAL (t)) == COMPOUND_EXPR;
> > +             if (need_paren)
> > +               pp_left_paren (pp);
> > +             dump_generic_node (pp, DECL_INITIAL (t), spc, flags, false);
> > +             if (need_paren)
> > +               pp_right_paren (pp);
> > +           }
> >           else
> >             pp_string (pp, "<<< omitted >>>");
> >         }
> > --
> > 2.43.0
> >

^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2024-05-03 22:12 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2024-05-02 21:39 [PATCH 1/3] Fix printing COMPOUND_EXPR in .original [PR23872] Andrew Pinski
2024-05-02 21:39 ` [PATCH 2/3] Improve DECL_EXPR printing " Andrew Pinski
2024-05-03 11:34   ` Richard Biener
2024-05-03 22:11     ` Andrew Pinski
2024-05-02 21:39 ` [PATCH 3/3] Add parentheses around DECL_INIT for " Andrew Pinski
2024-05-03 11:40   ` Richard Biener
2024-05-03 22:12     ` Andrew Pinski
2024-05-03 11:29 ` [PATCH 1/3] Fix printing COMPOUND_EXPR in " Richard Biener

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).