Hi!

The following testcase FAILs -fcompare-debug, because one COND_EXPR
branch from the FE during gimplifications is just <nop_expr <integer_cst>>
which doesn't have TREE_SIDE_EFFECTS, but for -gstatement-frontiers it
is a STATEMENT_LIST which contains # DEBUG BEGIN_STMT and that <nop_expr
<integer_cst>>.  Neither # DEBUG BEGIN_STMT nor that NOP_EXPR have
TREE_SIDE_EFFECTS, but STATEMENT_LIST has TREE_SIDE_EFFECTS already from
make_node and the gimplifier (and apparently the C++ FE too) checks
just that bit.  With { { { 0; } { 1; } { 2; } { 3; } } } one can end up
with quite large STATEMENT_LIST subtrees which in reality still don't
have any side-effects.
Maintaining accurate TREE_SIDE_EFFECTS bit on STATEMENT_LIST would be hard,
if we would only add and never remove statements, then we could just clear
it during make_node and set it whenever adding TREE_SIDE_EFFECTS statement
into it, but as soon as we sometimes remove from STATEMENT_LIST, or merge
STATEMENT_LISTs etc., maintaining this is too IMHO expensive, especially
when we usually just will not care about it.
So, I think it is better to just compute this lazily in the few spots where
we are interested about this, in real-world testcases most of the
STATEMENT_LISTs will have side-effects and should find them pretty early
when walking the tree.
As a side-effect, this patch will handle those
{ { { 0; } { 1; } { 2; } { 3; } } } and similar then/else statement lists
better.

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

2017-12-14  Jakub Jelinek  <ja...@redhat.com>

        PR debug/83419
        * tree.h (statement_with_side_effects_p): Declare.
        * tree.c (statement_with_side_effects_p): New function.
        * gimplify.c (shortcut_cond_expr, gimplify_cond_expr): Use it.

        * cp-gimplify.c (genericize_if_stmt, gimplify_expr_stmt,
        cp_fold): Use statement_with_side_effects_p instead of
        just TREE_SIDE_EFFECTS.

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

--- gcc/tree.h.jj       2017-12-12 09:48:15.000000000 +0100
+++ gcc/tree.h  2017-12-14 13:22:34.157858781 +0100
@@ -4780,6 +4780,7 @@ extern tree obj_type_ref_class (const_tr
 extern bool types_same_for_odr (const_tree type1, const_tree type2,
                                bool strict=false);
 extern bool contains_bitfld_component_ref_p (const_tree);
+extern bool statement_with_side_effects_p (tree);
 extern bool block_may_fallthru (const_tree);
 extern void using_eh_for_cleanups (void);
 extern bool using_eh_for_cleanups_p (void);
--- gcc/tree.c.jj       2017-12-12 09:48:15.000000000 +0100
+++ gcc/tree.c  2017-12-14 13:21:25.857752480 +0100
@@ -12296,6 +12296,26 @@ contains_bitfld_component_ref_p (const_t
   return false;
 }
 
+/* Return true if STMT has side-effects.  This is like
+   TREE_SIDE_EFFECTS (stmt), except it returns false for NULL and if STMT
+   is a STATEMENT_LIST, it recurses on the statements.  */
+
+bool
+statement_with_side_effects_p (tree stmt)
+{
+  if (stmt == NULL_TREE)
+    return false;
+  if (TREE_CODE (stmt) != STATEMENT_LIST)
+    return TREE_SIDE_EFFECTS (stmt);
+
+  for (tree_stmt_iterator i = tsi_start (stmt);
+       !tsi_end_p (i); tsi_next (&i))
+    if (statement_with_side_effects_p (tsi_stmt (i)))
+      return true;
+
+  return false;
+}
+
 /* Try to determine whether a TRY_CATCH expression can fall through.
    This is a subroutine of block_may_fallthru.  */
 
--- gcc/gimplify.c.jj   2017-12-14 11:53:34.907142223 +0100
+++ gcc/gimplify.c      2017-12-14 13:18:19.261184074 +0100
@@ -3637,8 +3637,8 @@ shortcut_cond_expr (tree expr)
   tree *true_label_p;
   tree *false_label_p;
   bool emit_end, emit_false, jump_over_else;
-  bool then_se = then_ && TREE_SIDE_EFFECTS (then_);
-  bool else_se = else_ && TREE_SIDE_EFFECTS (else_);
+  bool then_se = statement_with_side_effects_p (then_);
+  bool else_se = statement_with_side_effects_p (else_);
 
   /* First do simple transformations.  */
   if (!else_se)
@@ -3656,7 +3656,7 @@ shortcut_cond_expr (tree expr)
          if (rexpr_has_location (pred))
            SET_EXPR_LOCATION (expr, rexpr_location (pred));
          then_ = shortcut_cond_expr (expr);
-         then_se = then_ && TREE_SIDE_EFFECTS (then_);
+         then_se = statement_with_side_effects_p (then_);
          pred = TREE_OPERAND (pred, 0);
          expr = build3 (COND_EXPR, void_type_node, pred, then_, NULL_TREE);
          SET_EXPR_LOCATION (expr, locus);
@@ -3678,7 +3678,7 @@ shortcut_cond_expr (tree expr)
          if (rexpr_has_location (pred))
            SET_EXPR_LOCATION (expr, rexpr_location (pred));
          else_ = shortcut_cond_expr (expr);
-         else_se = else_ && TREE_SIDE_EFFECTS (else_);
+         else_se = statement_with_side_effects_p (else_);
          pred = TREE_OPERAND (pred, 0);
          expr = build3 (COND_EXPR, void_type_node, pred, NULL_TREE, else_);
          SET_EXPR_LOCATION (expr, locus);
@@ -3982,9 +3982,9 @@ gimplify_cond_expr (tree *expr_p, gimple
          if (gimplify_ctxp->allow_rhs_cond_expr
              /* If either branch has side effects or could trap, it can't be
                 evaluated unconditionally.  */
-             && !TREE_SIDE_EFFECTS (then_)
+             && !statement_with_side_effects_p (then_)
              && !generic_expr_could_trap_p (then_)
-             && !TREE_SIDE_EFFECTS (else_)
+             && !statement_with_side_effects_p (else_)
              && !generic_expr_could_trap_p (else_))
            return gimplify_pure_cond_expr (expr_p, pre_p);
 
--- gcc/cp/cp-gimplify.c.jj     2017-12-05 10:16:48.000000000 +0100
+++ gcc/cp/cp-gimplify.c        2017-12-14 13:24:08.373614768 +0100
@@ -176,9 +176,9 @@ genericize_if_stmt (tree *stmt_p)
   if (!else_)
     else_ = build_empty_stmt (locus);
 
-  if (integer_nonzerop (cond) && !TREE_SIDE_EFFECTS (else_))
+  if (integer_nonzerop (cond) && !statement_with_side_effects_p (else_))
     stmt = then_;
-  else if (integer_zerop (cond) && !TREE_SIDE_EFFECTS (then_))
+  else if (integer_zerop (cond) && !statement_with_side_effects_p (then_))
     stmt = else_;
   else
     stmt = build3 (COND_EXPR, void_type_node, cond, then_, else_);
@@ -424,7 +424,7 @@ gimplify_expr_stmt (tree *stmt_p)
      gimplification.  */
   if (stmt && warn_unused_value)
     {
-      if (!TREE_SIDE_EFFECTS (stmt))
+      if (!statement_with_side_effects_p (stmt))
        {
          if (!IS_EMPTY_STMT (stmt)
              && !VOID_TYPE_P (TREE_TYPE (stmt))
@@ -2096,7 +2096,7 @@ cp_fold (tree x)
       /* Strip CLEANUP_POINT_EXPR if the expression doesn't have side
         effects.  */
       r = cp_fold_rvalue (TREE_OPERAND (x, 0));
-      if (!TREE_SIDE_EFFECTS (r))
+      if (!statement_with_side_effects_p (r))
        x = r;
       break;
 
--- gcc/testsuite/gcc.dg/pr83419.c.jj   2017-12-14 13:14:45.520969384 +0100
+++ gcc/testsuite/gcc.dg/pr83419.c      2017-12-14 13:14:45.520969384 +0100
@@ -0,0 +1,16 @@
+/* PR debug/83419 */
+/* { dg-do compile } */
+/* { dg-options "-O2 -fcompare-debug" } */
+
+int a, b;
+void foo (int, ...);
+
+void
+bar (void)
+{
+  if (a || 1 == b)
+    foo (1);
+  else
+    0;
+  foo (1, 0);
+}

        Jakub

Reply via email to