The following implements a prototype for additional checking around
SSA immediate use iteration. Specifically this guards immediate
use list modifications inside a FOR_EACH_IMM_USE_STMT iteration
to be restricted to uses involving the actual stmt. It likewise
prohibits FOR_EACH_IMM_USE_STMT from inside another such iteration.
Checking is performend at FOR_EACH_IMM_USE_STMT start and the
iteration tracks the actual stmt being iterated in the 'use'
field of the SSA name immediate use field, enforcing that in
delink_imm_use (currently, as the main guard against removing
the immediate use following the current stmt set).
FOR_EACH_IMM_USE_FAST is out of scope for the moment, we'd need
more space for state. Likewise this is a prototype as it re-uses
the SSA name immediate use nodes 'use' field and thus had to disable
checking around the immediate use health. For FOR_EACH_IMM_USE_FAST
I'd like to have a flag indicating a fast iteration is taking place
(which necessarily would have to be a 'depth'). Starting a
FOR_EACH_IMM_USE_STMT should only be with depth == 0. And with
depth != 0 we do not want to change the immediate use list.
The patch depends on the previous one removing the fake sentinel
immediate use entry. The patch triggers issues elsewhere, like
for gcc.dg/torture/20181029-1.c, see 2/3 or during bootstrap,
see 3/3. The series does not bootstrap (more issues).
What this basically shows is that while the sentinel used with
FOR_EACH_IMM_USE_STMT ensures iteration itself doesn't break,
we do have cases where we disrupt this iteration in a way that
can cause stmts to be missed in the iteration (or selective uses
on one stmt for a nested FOR_EACH_IMM_USE_ON_STMT). And that
can be quite unexpected as the forwprop case shows (one of the
remaining update_stmt in that loop is still a problem).
Most definitely the dependent patch removing the sentinel will
eventually cause iteration to "break", either running off list
and crashing or re-iterating parts of the uses. But I also think
the current situation isn't sustainable (and the original issue
with ranger still exists). Also FOR_EACH_IMM_USE_FAST can be
similarly affected in case we go off to code updating a random
other stmt - like in forwprop if you release an unrelated
SSA name it might insert a debug stmt and update all of that
names uses and its use stmts, if that also has a use of the
fast iteration SSA name and this happens to be the one that
is queued as "next" then you'll visit some uses twice due
to update_stmt removing & re-inserting SSA uses. A use
might also vanish completely (and crash the iteration) if
downthread a stmt with a use is folded.
I'm not sure what the take away is here - immediate uses are bad?
Any comments?
Thanks,
Richard.
* ssa-iterators.h (imm_use_iterator::name): Add.
(delink_imm_use): When in a FOR_EACH_IMM_USE_STMT iteration
enforce we only remove uses from the current stmt.
(end_imm_use_stmt_traverse): Reset current stmt.
(first_imm_use_stmt): Assert no FOR_EACH_IMM_USE_STMT on
var is in progress. Set the current stmt.
(next_imm_use_stmt): Set the current stmt.
* tree-ssa-operands.cc (verify_imm_links): Disable assert.
* tree-ssa.cc (verify_use): Disable immediate use list
health check.
* tree-ssanames.cc (init_ssa_name_imm_use): Initialize
use to a value that is non-NULL and crashes on access.
---
gcc/ssa-iterators.h | 17 ++++++++++++++++-
gcc/tree-ssa-operands.cc | 2 +-
gcc/tree-ssa.cc | 2 ++
gcc/tree-ssanames.cc | 2 +-
4 files changed, 20 insertions(+), 3 deletions(-)
diff --git a/gcc/ssa-iterators.h b/gcc/ssa-iterators.h
index 77ecfac1993..25569a79570 100644
--- a/gcc/ssa-iterators.h
+++ b/gcc/ssa-iterators.h
@@ -59,6 +59,7 @@ struct imm_use_iterator
/* This is the next ssa_name to visit. IMM_USE may get removed before
the next one is traversed to, so it must be cached early. */
ssa_use_operand_t *next_imm_name;
+ tree name;
};
@@ -246,6 +247,15 @@ delink_imm_use (ssa_use_operand_t *linknode)
if (linknode->prev == NULL)
return;
+ if (linknode->loc.stmt
+ /* update_stmt on constant/removed uses. */
+ && USE_FROM_PTR (linknode)
+ && TREE_CODE (USE_FROM_PTR (linknode)) == SSA_NAME)
+ {
+ use_operand_p head = &(SSA_NAME_IMM_USE_NODE (USE_FROM_PTR (linknode)));
+ gcc_assert (head->use == (tree *)0x1u
+ || head->use == (tree *)linknode->loc.stmt);
+ }
linknode->prev->next = linknode->next;
linknode->next->prev = linknode->prev;
linknode->prev = NULL;
@@ -841,8 +851,9 @@ end_imm_use_stmt_p (const imm_use_iterator *imm)
placeholder node from the list. */
inline void
-end_imm_use_stmt_traverse (imm_use_iterator *)
+end_imm_use_stmt_traverse (imm_use_iterator *imm)
{
+ SSA_NAME_IMM_USE_NODE (imm->name).use = (tree *)0x1u;
}
/* Immediate use traversal of uses within a stmt require that all the
@@ -918,6 +929,7 @@ link_use_stmts_after (use_operand_p head, imm_use_iterator
*)
inline gimple *
first_imm_use_stmt (imm_use_iterator *imm, tree var)
{
+ gcc_assert (SSA_NAME_IMM_USE_NODE (var).use == (tree *)0x1u);
imm->end_p = &(SSA_NAME_IMM_USE_NODE (var));
imm->imm_use = imm->end_p->next;
imm->next_imm_name = NULL_USE_OPERAND_P;
@@ -925,12 +937,14 @@ first_imm_use_stmt (imm_use_iterator *imm, tree var)
/* next_stmt_use is used to point to the immediate use node after
the set of uses for the current stmt. */
imm->next_stmt_use = NULL_USE_OPERAND_P;
+ imm->name = var;
if (end_imm_use_stmt_p (imm))
return NULL;
imm->next_stmt_use = link_use_stmts_after (imm->imm_use, imm)->next;
+ SSA_NAME_IMM_USE_NODE (var).use = (tree *) USE_STMT (imm->imm_use);
return USE_STMT (imm->imm_use);
}
@@ -943,6 +957,7 @@ next_imm_use_stmt (imm_use_iterator *imm)
if (end_imm_use_stmt_p (imm))
return NULL;
+ SSA_NAME_IMM_USE_NODE (imm->name).use = (tree *) USE_STMT (imm->imm_use);
imm->next_stmt_use = link_use_stmts_after (imm->imm_use, imm)->next;
return USE_STMT (imm->imm_use);
}
diff --git a/gcc/tree-ssa-operands.cc b/gcc/tree-ssa-operands.cc
index a5970ac7b71..da0ba7ae017 100644
--- a/gcc/tree-ssa-operands.cc
+++ b/gcc/tree-ssa-operands.cc
@@ -1213,7 +1213,7 @@ verify_imm_links (FILE *f, tree var)
gcc_assert (TREE_CODE (var) == SSA_NAME);
list = &(SSA_NAME_IMM_USE_NODE (var));
- gcc_assert (list->use == NULL);
+ //gcc_assert (list->use == NULL);
if (list->prev == NULL)
{
diff --git a/gcc/tree-ssa.cc b/gcc/tree-ssa.cc
index fd714dacd27..53a1d15007c 100644
--- a/gcc/tree-ssa.cc
+++ b/gcc/tree-ssa.cc
@@ -908,6 +908,7 @@ verify_use (basic_block bb, basic_block def_bb,
use_operand_p use_p,
}
else
{
+#if 0
tree listvar;
if (use_p->prev->use == NULL)
listvar = use_p->prev->loc.ssa_name;
@@ -918,6 +919,7 @@ verify_use (basic_block bb, basic_block def_bb,
use_operand_p use_p,
error ("wrong immediate use list");
err = true;
}
+#endif
}
if (err)
diff --git a/gcc/tree-ssanames.cc b/gcc/tree-ssanames.cc
index dcf8da56272..d53d344eccb 100644
--- a/gcc/tree-ssanames.cc
+++ b/gcc/tree-ssanames.cc
@@ -335,7 +335,7 @@ init_ssa_name_imm_use (tree name)
{
use_operand_p imm;
imm = &(SSA_NAME_IMM_USE_NODE (name));
- imm->use = NULL;
+ imm->use = (tree *)0x1u;
imm->prev = imm;
imm->next = imm;
imm->loc.ssa_name = name;
--
2.51.0
