On Tue, 4 Nov 2025, Andrew MacLeod wrote:

> 
> On 11/4/25 08:43, Richard Biener wrote:
> > The following implements additional checking around
> > SSA immediate use iteration.  Specifically this prevents
> >
> >   - any nesting of FOR_EACH_IMM_USE_STMT inside another iteration
> >     via FOR_EACH_IMM_USE_STMT or FOR_EACH_IMM_USE_FAST when iterating
> >     on the same SSA name
> >
> >   - modification (for now unlinking of immediate uses) of a SSA
> >     immediate use list when a fast iteration of the immediate uses
> >     of the SSA name is active
> >
> >   - modification (for now unlinking of immediate uses) of the immediate
> >     use list outside of the block of uses for the currently active stmt
> >     of an ongoing FOR_EACH_IMM_USE_STMT of the SSA name
> >
> > To implement this additional bookkeeping members are put into the
> > SSA name structure when ENABLE_GIMPLE_CHECKING is active.  I have
> > kept the existing consistency checking of the fast iterator.
> >
> > The patch depends on the previous one removing the fake sentinel
> > immediate use entry.  Depends only in sofar that it doesn't apply
> > on trunk directly at the moment.
> >
> > Boostrapped and tested on x86_64-unknown-linux-gnu, ontop of
> > the other patches.
> >
> > Any further comments?  I've seen no comment on the very first
> > change yet which was removal of the sentinel element (to fix
> > the ranger interaction)?
> 
> I only noticed that yesterday :-)
> 
> The sentinels purpose was to make sure adjusting a stmt would be safe when
> things are removed. It serves as a guarantee the next pointer will be valid.  
> I am not opposed to replacing it with something more modern.. (that code is
> about 20 years old) as long as there are enough guardrails to deal with the
> additional case(s) that would be introduce.  In particular, the sentinel
> allows the remainder of the use list to change, including the thing it points
> to.       If the pointer to next is stashed before any updates, there is a
> danger what it points to may change... so that would have to be dissallowed
> and verified.
> 
> I think your listed guardrails should accomplish that.  They will trap in any
> place that attempts to make those modifications? which means those passes can
> then be fixed their poor use of immediate uses...

Yes.  The danger is of course we uncover those only very slowly.  But
we can also audit all FOR_EACH_IMM_USE_STMT uses and replace most.  There
are "only" 97 ...

> If this is going in shortly, we don't need my FAST patch that skips the soon
> to be nonexistent sentinel eh?.

Correct.  If we're OK to play whack-a-mole for a bit we can go for it.
I have some documentation updates to do to document the constraints
of course.

Thanks,
Richard.

Reply via email to