Re: [PATCH] c++: Fix a -fcompare-debug issue with DEBUG_BEGIN_STMT stmts in STATEMENT_LISTs [PR94272]

2020-03-25 Thread Jason Merrill via Gcc-patches

On 3/24/20 3:58 AM, Jakub Jelinek wrote:

+ 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)))


How about (nondebug_stmts > 1 || TREE_SIDE_EFFECTS (tsi_stmt (i))) here...


+   clear_side_effects = false;
+   }
+ if (nondebug_stmts < 2 && clear_side_effects)


So we don't need to look at nondebug_stmts here?  OK with that change.

Jason



[PATCH] c++: Fix a -fcompare-debug issue with DEBUG_BEGIN_STMT stmts in STATEMENT_LISTs [PR94272]

2020-03-24 Thread Jakub Jelinek via Gcc-patches
Hi!

The following testcase FAILs with -fcompare-debug.  The problem is that
the C++ FE initially uses IF_STMTs, tcc_statement which default to
TREE_SIDE_EFFECTS set, but later on is genericized into COND_EXPRs,
tcc_expression which default to TREE_SIDE_EFFECTS ored from all 3 operands.
Furthermore, with -g we emit by default DEBUG_BEGIN_STMTs (TREE_SIDE_EFFECTS
clear) and so end up with a STATEMENT_LIST containing DEBUG_BEGIN_STMT
+ e.g. the IF_STMT, while with -g0 we would end up with just the IF_STMT
alone and in that case there is no STATEMENT_LIST wrapping it.

Now, the STATEMENT_LIST has TREE_SIDE_EFFECTS set to match the IF_STMT,
but if none of the 3 operands (condition and both branches) have
TREE_SIDE_EFFECTS, genericize_if_stmt will replace the IF_STMT with
COND_EXPR without TREE_SIDE_EFFECTS, but with -g only STATEMENT_LIST
wrapping it will keep TREE_SIDE_EFFECTS.  Then during gimplification,
shortcut_cond_expr checks TREE_SIDE_EFFECTS of the operands and as it
is differennt between -g and -g0, will generate different code.

The following patch attempts to fix this by clearing TREE_SIDE_EFFECTS
on STATEMENT_LISTs that initially have it set and contain only
DEBUG_BEGIN_STMT or at most one other statement that lost TREE_SIDE_EFFECTS
during the genericization.

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

2020-03-24  Jakub Jelinek  

PR c++/94272
* cp-gimplify.c (cp_genericize_r): Handle STATEMENT_LIST.

* g++.dg/debug/pr94272.C: New test.

--- gcc/cp/cp-gimplify.c.jj 2020-03-20 09:11:36.276902935 +0100
+++ gcc/cp/cp-gimplify.c2020-03-23 14:50:08.354821919 +0100
@@ -1754,6 +1754,36 @@ 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 (nondebug_stmts < 2 && clear_side_effects)
+   TREE_SIDE_EFFECTS (stmt) = 0;
+ *walk_subtrees = 0;
+   }
+  break;
+
 default:
   if (IS_TYPE_OR_DECL_P (stmt))
*walk_subtrees = 0;
--- gcc/testsuite/g++.dg/debug/pr94272.C.jj 2020-03-23 15:04:49.037693587 
+0100
+++ gcc/testsuite/g++.dg/debug/pr94272.C2020-03-23 15:04:04.109363546 
+0100
@@ -0,0 +1,14 @@
+// PR c++/94272
+// { dg-do compile }
+// { dg-options "-O2 -fnon-call-exceptions -fcompare-debug" }
+
+int *c, d, *e;
+
+void
+foo ()
+{
+  if (c && d)
+;
+  else if (*e)
+;
+}

Jakub