On Fri, Dec 15, 2017 at 09:34:44AM +0100, Richard Biener wrote:
> I don't like this too much. Iff then we should do "real" lazy
> computation, like adding a TREE_SIDE_EFFECTS_VALID flag on STATEMENT_LIST,
> keeping TREE_SIDE_EFFECTS up-to-date when easily possible and when doing
> the expensive thing cache the result. That said, I'm not convinced
> this will fix -fcompare-debug issues for good. Is it really necessary
> to introduce this IL difference so early and in such an intrusive way?
>
> Can't we avoid adding # DEBUG BEGIN_STMT when there's not already
> a STATEMENT_LIST around for example?
The STATEMENT_LIST handling is a complete mess.
E.g.
tree
alloc_stmt_list (void)
{
tree list;
if (!vec_safe_is_empty (stmt_list_cache))
{
list = stmt_list_cache->pop ();
memset (list, 0, sizeof (struct tree_base));
TREE_SET_CODE (list, STATEMENT_LIST);
}
else
list = make_node (STATEMENT_LIST);
Because make_node (STATEMENT_LIST) sets TREE_SIDE_EFFECTS, but the above
memset clears it, we start with inconsistent TREE_SIDE_EFFECTS from the
beginning.
I've tried to track TREE_SIDE_EFFECTS precisely, as below, but that seems to
break completely the statement expression handling, in particular
make check-gcc check-g++ RUNTESTFLAGS="compile.exp=20030530-3.c
debug.exp='20020224-1.c 20030605-1.c' dg.exp='Wduplicated-branches-1.c
compare2.c gnu89-const-expr-1.c pr64637.c pr68412-2.c stmt-expr-*.c pr56419.C'
lto.exp=pr83388* noncompile.exp=971104-1.c dg-torture.exp=pr51071.c
tree-ssa.exp='20030711-2.c 20030714-1.c 20030731-1.c' vect.exp=pr33833.c
tm.exp=pr56419.C"
all FAILs because of that, e.g.
gcc/testsuite/gcc.c-torture/compile/20030530-3.c:14:8: error: void value not
ignored as it ought to be
I've also tried to track instead of tracking TREE_SIDE_EFFECTS precisely
track just ignore DEBUG_BEGIN_STMTs in the processing, but that
unfortunately doesn't help for the testcase included in the patch (attached
patch).
2017-12-18 Jakub Jelinek <ja...@redhat.com>
PR debug/83419
* tree-iterator.c (alloc_stmt_list): Clear TREE_SIDE_EFFECTS for a new
node.
(tsi_link_after): Set TREE_SIDE_EFFECTS when adding t with
TREE_SIDE_EFFECTS.
(tsi_link_before): Likewise. Formatting fix.
(tsi_delink): Recompute TREE_SIDE_EFFECTS on removal.
* gimplify.c (gimplify_statement_list): Clear TREE_SIDE_EFFECTS.
* gcc.dg/pr83419.c: New test.
--- gcc/tree-iterator.c.jj 2017-12-12 09:48:26.000000000 +0100
+++ gcc/tree-iterator.c 2017-12-18 10:13:58.776212769 +0100
@@ -41,7 +41,10 @@ alloc_stmt_list (void)
TREE_SET_CODE (list, STATEMENT_LIST);
}
else
- list = make_node (STATEMENT_LIST);
+ {
+ list = make_node (STATEMENT_LIST);
+ TREE_SIDE_EFFECTS (list) = 0;
+ }
TREE_TYPE (list) = void_type_node;
return list;
}
@@ -137,7 +140,7 @@ tsi_link_before (tree_stmt_iterator *i,
tail = head;
}
- if (TREE_CODE (t) != DEBUG_BEGIN_STMT)
+ if (TREE_SIDE_EFFECTS (t))
TREE_SIDE_EFFECTS (i->container) = 1;
cur = i->ptr;
@@ -157,9 +160,9 @@ tsi_link_before (tree_stmt_iterator *i,
{
head->prev = STATEMENT_LIST_TAIL (i->container);
if (head->prev)
- head->prev->next = head;
+ head->prev->next = head;
else
- STATEMENT_LIST_HEAD (i->container) = head;
+ STATEMENT_LIST_HEAD (i->container) = head;
STATEMENT_LIST_TAIL (i->container) = tail;
}
@@ -214,7 +217,7 @@ tsi_link_after (tree_stmt_iterator *i, t
tail = head;
}
- if (TREE_CODE (t) != DEBUG_BEGIN_STMT)
+ if (TREE_SIDE_EFFECTS (t))
TREE_SIDE_EFFECTS (i->container) = 1;
cur = i->ptr;
@@ -275,10 +278,28 @@ tsi_delink (tree_stmt_iterator *i)
else
STATEMENT_LIST_TAIL (i->container) = prev;
- if (!next && !prev)
- TREE_SIDE_EFFECTS (i->container) = 0;
-
i->ptr = next;
+
+ if (TREE_SIDE_EFFECTS (i->container))
+ {
+ while (next || prev)
+ {
+ if (next)
+ {
+ if (TREE_SIDE_EFFECTS (next->stmt))
+ break;
+ next = next->next;
+ }
+ if (prev)
+ {
+ if (TREE_SIDE_EFFECTS (prev->stmt))
+ break;
+ prev = prev->prev;
+ }
+ }
+ if (next == NULL && prev == NULL)
+ TREE_SIDE_EFFECTS (i->container) = 0;
+ }
}
/* Return the first expression in a sequence of COMPOUND_EXPRs, or in
--- gcc/gimplify.c.jj 2017-12-14 21:11:40.000000000 +0100
+++ gcc/gimplify.c 2017-12-18 10:17:07.922556324 +0100
@@ -1763,6 +1763,9 @@ gimplify_statement_list (tree *expr_p, g
tree_stmt_iterator i = tsi_start (*expr_p);
+ /* No need to recompute TREE_SIDE_EFFECTS on removal, we are going to
+ delink everything. */
+ TREE_SIDE_EFFECTS (*expr_p) = 0;
while (!tsi_end_p (i))
{
gimplify_stmt (tsi_stmt_ptr (i), pre_p);
--- gcc/testsuite/gcc.dg/pr83419.c.jj 2017-12-18 10:08:24.214911480 +0100
+++ gcc/testsuite/gcc.dg/pr83419.c 2017-12-18 10:08:24.214911480 +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
2017-12-18 Jakub Jelinek <ja...@redhat.com>
PR debug/83419
* tree-iterator.c: Include options.h.
(alloc_stmt_list): Clear TREE_SIDE_EFFECTS for a new node.
(tsi_link_after): Set TREE_SIDE_EFFECTS when adding STATEMENT_LIST with
TREE_SIDE_EFFECTS or non-DEBUG_BEGIN_STMT.
(tsi_link_before): Likewise. Formatting fix.
(tsi_delink): Recompute TREE_SIDE_EFFECTS on removal.
* gimplify.c (gimplify_statement_list): Clear TREE_SIDE_EFFECTS.
* gcc.dg/pr83419.c: New test.
--- gcc/tree-iterator.c.jj 2017-12-12 09:48:26.000000000 +0100
+++ gcc/tree-iterator.c 2017-12-18 13:44:02.330719440 +0100
@@ -23,6 +23,7 @@ along with GCC; see the file COPYING3.
#include "coretypes.h"
#include "tree.h"
#include "tree-iterator.h"
+#include "options.h"
/* This is a cache of STATEMENT_LIST nodes. We create and destroy them
@@ -41,7 +42,10 @@ alloc_stmt_list (void)
TREE_SET_CODE (list, STATEMENT_LIST);
}
else
- list = make_node (STATEMENT_LIST);
+ {
+ list = make_node (STATEMENT_LIST);
+ TREE_SIDE_EFFECTS (list) = 0;
+ }
TREE_TYPE (list) = void_type_node;
return list;
}
@@ -127,6 +131,8 @@ tsi_link_before (tree_stmt_iterator *i,
gcc_assert (head == tail);
return;
}
+ if (TREE_SIDE_EFFECTS (t))
+ TREE_SIDE_EFFECTS (i->container) = 1;
}
else
{
@@ -135,11 +141,10 @@ tsi_link_before (tree_stmt_iterator *i,
head->next = NULL;
head->stmt = t;
tail = head;
+ if (TREE_CODE (t) != DEBUG_BEGIN_STMT)
+ TREE_SIDE_EFFECTS (i->container) = 1;
}
- if (TREE_CODE (t) != DEBUG_BEGIN_STMT)
- TREE_SIDE_EFFECTS (i->container) = 1;
-
cur = i->ptr;
/* Link it into the list. */
@@ -157,9 +162,9 @@ tsi_link_before (tree_stmt_iterator *i,
{
head->prev = STATEMENT_LIST_TAIL (i->container);
if (head->prev)
- head->prev->next = head;
+ head->prev->next = head;
else
- STATEMENT_LIST_HEAD (i->container) = head;
+ STATEMENT_LIST_HEAD (i->container) = head;
STATEMENT_LIST_TAIL (i->container) = tail;
}
@@ -204,6 +209,9 @@ tsi_link_after (tree_stmt_iterator *i, t
gcc_assert (head == tail);
return;
}
+
+ if (TREE_SIDE_EFFECTS (t))
+ TREE_SIDE_EFFECTS (i->container) = 1;
}
else
{
@@ -212,10 +220,10 @@ tsi_link_after (tree_stmt_iterator *i, t
head->next = NULL;
head->stmt = t;
tail = head;
- }
- if (TREE_CODE (t) != DEBUG_BEGIN_STMT)
- TREE_SIDE_EFFECTS (i->container) = 1;
+ if (TREE_CODE (t) != DEBUG_BEGIN_STMT)
+ TREE_SIDE_EFFECTS (i->container) = 1;
+ }
cur = i->ptr;
@@ -275,10 +283,30 @@ tsi_delink (tree_stmt_iterator *i)
else
STATEMENT_LIST_TAIL (i->container) = prev;
- if (!next && !prev)
- TREE_SIDE_EFFECTS (i->container) = 0;
-
i->ptr = next;
+
+ if (!MAY_HAVE_DEBUG_MARKER_STMTS)
+ TREE_SIDE_EFFECTS (i->container) = 0;
+ else if (TREE_SIDE_EFFECTS (i->container))
+ {
+ while (next || prev)
+ {
+ if (next)
+ {
+ if (TREE_CODE (next->stmt) != DEBUG_BEGIN_STMT)
+ break;
+ next = next->next;
+ }
+ if (prev)
+ {
+ if (TREE_CODE (prev->stmt) != DEBUG_BEGIN_STMT)
+ break;
+ prev = prev->prev;
+ }
+ }
+ if (next == NULL && prev == NULL)
+ TREE_SIDE_EFFECTS (i->container) = 0;
+ }
}
/* Return the first expression in a sequence of COMPOUND_EXPRs, or in
--- gcc/gimplify.c.jj 2017-12-14 21:11:40.000000000 +0100
+++ gcc/gimplify.c 2017-12-18 10:17:07.922556324 +0100
@@ -1763,6 +1763,9 @@ gimplify_statement_list (tree *expr_p, g
tree_stmt_iterator i = tsi_start (*expr_p);
+ /* No need to recompute TREE_SIDE_EFFECTS on removal, we are going to
+ delink everything. */
+ TREE_SIDE_EFFECTS (*expr_p) = 0;
while (!tsi_end_p (i))
{
gimplify_stmt (tsi_stmt_ptr (i), pre_p);
--- gcc/testsuite/gcc.dg/pr83419.c.jj 2017-12-18 10:08:24.214911480 +0100
+++ gcc/testsuite/gcc.dg/pr83419.c 2017-12-18 10:08:24.214911480 +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);
+}
