On Mon, Nov 3, 2025 at 7:20 AM Richard Biener <[email protected]> wrote:
>
> 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).
I like it because at least now I know what might be valid or not while
using FOR_EACH_IMM_USE_ON_STMT.
>
> 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).
optimize_agr_copyprop_1 might have an issue there. Likewise
optimize_agr_copyprop_arg. Both call update_stmt while under
FOR_EACH_IMM_USE_STMT.
Thanks,
Andrew
>
> 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
>
