This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
[PATCH] Fix -fcompare-debug due to DEBUG_BEGIN_STMTs (PR debug/83419)
- From: Jakub Jelinek <jakub at redhat dot com>
- To: Richard Biener <rguenther at suse dot de>, Jeff Law <law at redhat dot com>, Alexandre Oliva <aoliva at redhat dot com>
- Cc: gcc-patches at gcc dot gnu dot org
- Date: Thu, 14 Dec 2017 21:01:35 +0100
- Subject: [PATCH] Fix -fcompare-debug due to DEBUG_BEGIN_STMTs (PR debug/83419)
- Authentication-results: sourceware.org; auth=none
- Reply-to: Jakub Jelinek <jakub at redhat dot com>
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 <jakub@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