diff mbox series

c-family: Fix PR94272 -fcompare-debug issue even for C [PR99230]

Message ID 20210318084810.GP231854@tucnak
State New
Headers show
Series c-family: Fix PR94272 -fcompare-debug issue even for C [PR99230] | expand

Commit Message

Jakub Jelinek March 18, 2021, 8:48 a.m. UTC 
Hi!

The following testcase results in -fcompare-debug failure.
The problem is the similar like in PR94272
https://gcc.gnu.org/pipermail/gcc-patches/2020-March/542562.html
When genericizing, with -g0 we have just a TREE_SIDE_EFFECTS DO_STMT
in a branch of if, while with -g we have that wrapped into
TREE_SIDE_EFFECTS STATEMENT_LIST containing DEBUG_BEGIN_STMT and that
DO_STMT.
The do loop is empty with 0 condition, so c_genericize_control_stmt
turns it into an empty statement (without TREE_SIDE_EFFECTS).
For -g0 that means that suddenly the if branch doesn't have side effects
and is expanded differently.  But with -g we still have TREE_SIDE_EFFECTS
STATEMENT_LIST containing DEBUG_BEGIN_STMT and non-TREE_SIDE_EFFECTS stmt.
The following patch fixes that by detecting this case and removing
TREE_SIDE_EFFECTS.

And, so that we don't duplicate the same code, changes the C++ FE to
just call the c_genericize_control_stmt function that can now handle it.

Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?

2021-03-18  Jakub Jelinek  <jakub@redhat.com>

	PR debug/99230
	* c-gimplify.c (c_genericize_control_stmt): Handle STATEMENT_LIST.

	* cp-gimplify.c (cp_genericize_r) <case STATEMENT_LIST>: Remove
	special code, instead call c_genericize_control_stmt.

	* gcc.dg/pr99230.c: New test.


	Jakub

Comments

Jeff Law March 20, 2021, 3:31 p.m. UTC | #1 
On 3/18/2021 2:48 AM, Jakub Jelinek via Gcc-patches wrote:
> Hi!
>
> The following testcase results in -fcompare-debug failure.
> The problem is the similar like in PR94272
> https://gcc.gnu.org/pipermail/gcc-patches/2020-March/542562.html
> When genericizing, with -g0 we have just a TREE_SIDE_EFFECTS DO_STMT
> in a branch of if, while with -g we have that wrapped into
> TREE_SIDE_EFFECTS STATEMENT_LIST containing DEBUG_BEGIN_STMT and that
> DO_STMT.
> The do loop is empty with 0 condition, so c_genericize_control_stmt
> turns it into an empty statement (without TREE_SIDE_EFFECTS).
> For -g0 that means that suddenly the if branch doesn't have side effects
> and is expanded differently.  But with -g we still have TREE_SIDE_EFFECTS
> STATEMENT_LIST containing DEBUG_BEGIN_STMT and non-TREE_SIDE_EFFECTS stmt.
> The following patch fixes that by detecting this case and removing
> TREE_SIDE_EFFECTS.
>
> And, so that we don't duplicate the same code, changes the C++ FE to
> just call the c_genericize_control_stmt function that can now handle it.
>
> Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?
>
> 2021-03-18  Jakub Jelinek  <jakub@redhat.com>
>
> 	PR debug/99230
> 	* c-gimplify.c (c_genericize_control_stmt): Handle STATEMENT_LIST.
>
> 	* cp-gimplify.c (cp_genericize_r) <case STATEMENT_LIST>: Remove
> 	special code, instead call c_genericize_control_stmt.
>
> 	* gcc.dg/pr99230.c: New test.

OK

jeff
Jason Merrill March 22, 2021, 10:34 p.m. UTC | #2 
On 3/18/21 4:48 AM, Jakub Jelinek wrote:
> Hi!
> 
> The following testcase results in -fcompare-debug failure.
> The problem is the similar like in PR94272
> https://gcc.gnu.org/pipermail/gcc-patches/2020-March/542562.html
> When genericizing, with -g0 we have just a TREE_SIDE_EFFECTS DO_STMT
> in a branch of if, while with -g we have that wrapped into
> TREE_SIDE_EFFECTS STATEMENT_LIST containing DEBUG_BEGIN_STMT and that
> DO_STMT.
> The do loop is empty with 0 condition, so c_genericize_control_stmt
> turns it into an empty statement (without TREE_SIDE_EFFECTS).
> For -g0 that means that suddenly the if branch doesn't have side effects
> and is expanded differently.  But with -g we still have TREE_SIDE_EFFECTS
> STATEMENT_LIST containing DEBUG_BEGIN_STMT and non-TREE_SIDE_EFFECTS stmt.
> The following patch fixes that by detecting this case and removing
> TREE_SIDE_EFFECTS.
> 
> And, so that we don't duplicate the same code, changes the C++ FE to
> just call the c_genericize_control_stmt function that can now handle it.
> 
> Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?

The C++ change is OK.

> 2021-03-18  Jakub Jelinek  <jakub@redhat.com>
> 
> 	PR debug/99230
> 	* c-gimplify.c (c_genericize_control_stmt): Handle STATEMENT_LIST.
> 
> 	* cp-gimplify.c (cp_genericize_r) <case STATEMENT_LIST>: Remove
> 	special code, instead call c_genericize_control_stmt.
> 
> 	* gcc.dg/pr99230.c: New test.
> 
> --- gcc/c-family/c-gimplify.c.jj	2021-01-04 10:25:50.402102825 +0100
> +++ gcc/c-family/c-gimplify.c	2021-03-17 19:19:20.052702095 +0100
> @@ -497,6 +497,35 @@ c_genericize_control_stmt (tree *stmt_p,
>         genericize_omp_for_stmt (stmt_p, walk_subtrees, data, func, lh);
>         break;
>   
> +    case STATEMENT_LIST:
> +      if (TREE_SIDE_EFFECTS (stmt))
> +	{
> +	  tree_stmt_iterator i;
> +	  int nondebug_stmts = 0;
> +	  bool clear_side_effects = true;
> +	  /* Genericization can clear TREE_SIDE_EFFECTS, e.g. when
> +	     transforming an IF_STMT into COND_EXPR.  If such stmt
> +	     appears in a STATEMENT_LIST that contains only that
> +	     stmt and some DEBUG_BEGIN_STMTs, without -g where the
> +	     STATEMENT_LIST wouldn't be present at all the resulting
> +	     expression wouldn't have TREE_SIDE_EFFECTS set, so make sure
> +	     to clear it even on the STATEMENT_LIST in such cases.  */
> +	  for (i = tsi_start (stmt); !tsi_end_p (i); tsi_next (&i))
> +	    {
> +	      tree t = tsi_stmt (i);
> +	      if (TREE_CODE (t) != DEBUG_BEGIN_STMT && nondebug_stmts < 2)
> +		nondebug_stmts++;
> +	      walk_tree_1 (tsi_stmt_ptr (i), func, data, NULL, lh);
> +	      if (TREE_CODE (t) != DEBUG_BEGIN_STMT
> +		  && (nondebug_stmts > 1 || TREE_SIDE_EFFECTS (tsi_stmt (i))))
> +		clear_side_effects = false;
> +	    }
> +	  if (clear_side_effects)
> +	    TREE_SIDE_EFFECTS (stmt) = 0;
> +	  *walk_subtrees = 0;
> +	}
> +      break;
> +
>       default:
>         break;
>       }
> --- gcc/cp/cp-gimplify.c.jj	2021-03-04 09:42:27.602123905 +0100
> +++ gcc/cp/cp-gimplify.c	2021-03-17 19:38:42.743888589 +0100
> @@ -1464,35 +1464,6 @@ cp_genericize_r (tree *stmt_p, int *walk
>         walk_subtrees = 0;
>         break;
>   
> -    case STATEMENT_LIST:
> -      if (TREE_SIDE_EFFECTS (stmt))
> -	{
> -	  tree_stmt_iterator i;
> -	  int nondebug_stmts = 0;
> -	  bool clear_side_effects = true;
> -	  /* Genericization can clear TREE_SIDE_EFFECTS, e.g. when
> -	     transforming an IF_STMT into COND_EXPR.  If such stmt
> -	     appears in a STATEMENT_LIST that contains only that
> -	     stmt and some DEBUG_BEGIN_STMTs, without -g where the
> -	     STATEMENT_LIST wouldn't be present at all the resulting
> -	     expression wouldn't have TREE_SIDE_EFFECTS set, so make sure
> -	     to clear it even on the STATEMENT_LIST in such cases.  */
> -	  for (i = tsi_start (stmt); !tsi_end_p (i); tsi_next (&i))
> -	    {
> -	      tree t = tsi_stmt (i);
> -	      if (TREE_CODE (t) != DEBUG_BEGIN_STMT && nondebug_stmts < 2)
> -		nondebug_stmts++;
> -	      cp_walk_tree (tsi_stmt_ptr (i), cp_genericize_r, data, NULL);
> -	      if (TREE_CODE (t) != DEBUG_BEGIN_STMT
> -		  && (nondebug_stmts > 1 || TREE_SIDE_EFFECTS (tsi_stmt (i))))
> -		clear_side_effects = false;
> -	    }
> -	  if (clear_side_effects)
> -	    TREE_SIDE_EFFECTS (stmt) = 0;
> -	  *walk_subtrees = 0;
> -	}
> -      break;
> -
>       case OMP_DISTRIBUTE:
>         /* Need to explicitly instantiate copy ctors on class iterators of
>   	 composite distribute parallel for.  */
> @@ -1566,6 +1537,7 @@ cp_genericize_r (tree *stmt_p, int *walk
>       case OMP_SIMD:
>       case OMP_LOOP:
>       case OACC_LOOP:
> +    case STATEMENT_LIST:
>         /* These cases are handled by shared code.  */
>         c_genericize_control_stmt (stmt_p, walk_subtrees, data,
>   				 cp_genericize_r, cp_walk_subtrees);
> --- gcc/testsuite/gcc.dg/pr99230.c.jj	2021-03-17 19:34:26.633711074 +0100
> +++ gcc/testsuite/gcc.dg/pr99230.c	2021-03-17 19:33:30.210332887 +0100
> @@ -0,0 +1,40 @@
> +/* PR debug/99230 */
> +/* { dg-do compile } */
> +/* { dg-options "-O2 --param logical-op-non-short-circuit=0 -fcompare-debug --param=jump-table-max-growth-ratio-for-speed=5000" } */
> +
> +extern void fn2 (void);
> +extern void fn3 (int);
> +int a, b;
> +void
> +fn1 (void)
> +{
> +  int c;
> +  short d;
> +  switch (a) {
> +  case 22000:
> +    fn2 ();
> +  case 22300:
> +    b = 0;
> +  case 22600:
> +  case 22601:
> +  case 22900:
> +    fn3 (1);
> +  case 20100:
> +    fn3 (2);
> +  case 20200:
> +    fn3 (3);
> +  case 20300:
> +    fn3 (4);
> +  case 20400:
> +    fn3 (5);
> +  case 20310:
> +    fn3 (4);
> +  case 20410:
> +    fn3 (5);
> +  }
> +  if (d || c) {
> +    do
> +      ;
> +    while (0);
> +  }
> +}
> 
> 	Jakub
>
diff mbox series

Patch

--- gcc/c-family/c-gimplify.c.jj	2021-01-04 10:25:50.402102825 +0100
+++ gcc/c-family/c-gimplify.c	2021-03-17 19:19:20.052702095 +0100
@@ -497,6 +497,35 @@  c_genericize_control_stmt (tree *stmt_p,
       genericize_omp_for_stmt (stmt_p, walk_subtrees, data, func, lh);
       break;
 
+    case STATEMENT_LIST:
+      if (TREE_SIDE_EFFECTS (stmt))
+	{
+	  tree_stmt_iterator i;
+	  int nondebug_stmts = 0;
+	  bool clear_side_effects = true;
+	  /* Genericization can clear TREE_SIDE_EFFECTS, e.g. when
+	     transforming an IF_STMT into COND_EXPR.  If such stmt
+	     appears in a STATEMENT_LIST that contains only that
+	     stmt and some DEBUG_BEGIN_STMTs, without -g where the
+	     STATEMENT_LIST wouldn't be present at all the resulting
+	     expression wouldn't have TREE_SIDE_EFFECTS set, so make sure
+	     to clear it even on the STATEMENT_LIST in such cases.  */
+	  for (i = tsi_start (stmt); !tsi_end_p (i); tsi_next (&i))
+	    {
+	      tree t = tsi_stmt (i);
+	      if (TREE_CODE (t) != DEBUG_BEGIN_STMT && nondebug_stmts < 2)
+		nondebug_stmts++;
+	      walk_tree_1 (tsi_stmt_ptr (i), func, data, NULL, lh);
+	      if (TREE_CODE (t) != DEBUG_BEGIN_STMT
+		  && (nondebug_stmts > 1 || TREE_SIDE_EFFECTS (tsi_stmt (i))))
+		clear_side_effects = false;
+	    }
+	  if (clear_side_effects)
+	    TREE_SIDE_EFFECTS (stmt) = 0;
+	  *walk_subtrees = 0;
+	}
+      break;
+
     default:
       break;
     }
--- gcc/cp/cp-gimplify.c.jj	2021-03-04 09:42:27.602123905 +0100
+++ gcc/cp/cp-gimplify.c	2021-03-17 19:38:42.743888589 +0100
@@ -1464,35 +1464,6 @@  cp_genericize_r (tree *stmt_p, int *walk
       walk_subtrees = 0;
       break;
 
-    case STATEMENT_LIST:
-      if (TREE_SIDE_EFFECTS (stmt))
-	{
-	  tree_stmt_iterator i;
-	  int nondebug_stmts = 0;
-	  bool clear_side_effects = true;
-	  /* Genericization can clear TREE_SIDE_EFFECTS, e.g. when
-	     transforming an IF_STMT into COND_EXPR.  If such stmt
-	     appears in a STATEMENT_LIST that contains only that
-	     stmt and some DEBUG_BEGIN_STMTs, without -g where the
-	     STATEMENT_LIST wouldn't be present at all the resulting
-	     expression wouldn't have TREE_SIDE_EFFECTS set, so make sure
-	     to clear it even on the STATEMENT_LIST in such cases.  */
-	  for (i = tsi_start (stmt); !tsi_end_p (i); tsi_next (&i))
-	    {
-	      tree t = tsi_stmt (i);
-	      if (TREE_CODE (t) != DEBUG_BEGIN_STMT && nondebug_stmts < 2)
-		nondebug_stmts++;
-	      cp_walk_tree (tsi_stmt_ptr (i), cp_genericize_r, data, NULL);
-	      if (TREE_CODE (t) != DEBUG_BEGIN_STMT
-		  && (nondebug_stmts > 1 || TREE_SIDE_EFFECTS (tsi_stmt (i))))
-		clear_side_effects = false;
-	    }
-	  if (clear_side_effects)
-	    TREE_SIDE_EFFECTS (stmt) = 0;
-	  *walk_subtrees = 0;
-	}
-      break;
-
     case OMP_DISTRIBUTE:
       /* Need to explicitly instantiate copy ctors on class iterators of
 	 composite distribute parallel for.  */
@@ -1566,6 +1537,7 @@  cp_genericize_r (tree *stmt_p, int *walk
     case OMP_SIMD:
     case OMP_LOOP:
     case OACC_LOOP:
+    case STATEMENT_LIST:
       /* These cases are handled by shared code.  */
       c_genericize_control_stmt (stmt_p, walk_subtrees, data,
 				 cp_genericize_r, cp_walk_subtrees);
--- gcc/testsuite/gcc.dg/pr99230.c.jj	2021-03-17 19:34:26.633711074 +0100
+++ gcc/testsuite/gcc.dg/pr99230.c	2021-03-17 19:33:30.210332887 +0100
@@ -0,0 +1,40 @@ 
+/* PR debug/99230 */
+/* { dg-do compile } */
+/* { dg-options "-O2 --param logical-op-non-short-circuit=0 -fcompare-debug --param=jump-table-max-growth-ratio-for-speed=5000" } */
+
+extern void fn2 (void);
+extern void fn3 (int);
+int a, b;
+void
+fn1 (void)
+{
+  int c;
+  short d;
+  switch (a) {
+  case 22000:
+    fn2 ();
+  case 22300:
+    b = 0;
+  case 22600:
+  case 22601:
+  case 22900:
+    fn3 (1);
+  case 20100:
+    fn3 (2);
+  case 20200:
+    fn3 (3);
+  case 20300:
+    fn3 (4);
+  case 20400:
+    fn3 (5);
+  case 20310:
+    fn3 (4);
+  case 20410:
+    fn3 (5);
+  }
+  if (d || c) {
+    do
+      ;
+    while (0);
+  }
+}