On Mon, 27 Oct 2025, Tamar Christina wrote:
> > -----Original Message-----
> > From: Richard Biener <[email protected]>
> > Sent: 27 October 2025 09:06
> > To: Andrew Pinski <[email protected]>
> > Cc: Tamar Christina <[email protected]>; [email protected];
> > nd <[email protected]>
> > Subject: Re: [PATCH]middle-end: Don't ICE during gimple_build of an
> > incomplete PHI node
> >
> > On Sun, 26 Oct 2025, Andrew Pinski wrote:
> >
> > > On Sat, Oct 25, 2025 at 2:54 AM Tamar Christina
> > <[email protected]> wrote:
> > > >
> > > > The new building APIs e.g. gimple_build APIs at the moment ICE if you
> > > > try
> > to
> > > > build a PHI node without all the arguments set yet. However this
> > > > workflow
> > is
> > > > quite common, especially in the vectorizer.
> > > >
> > > > This change allows this case by just not folding the PHI is any
> > > > argument is
> > > > missing as we don't know the value, we also don't know if it's
> > > > nonnegative.
> > > >
> > > > Bootstrapped Regtested on aarch64-none-linux-gnu,
> > > > arm-none-linux-gnueabihf, x86_64-pc-linux-gnu
> > > > -m32, -m64 and no issues
> > > >
> > > > Ok for master?
> > >
> > > I ran into the same issue before and Richi rejected a similar patch; I
> > > was also checking if the ssa name was in the free list.
> > >
> > > See https://gcc.gnu.org/bugzilla/show_bug.cgi?id=120280#c10 and
> > afterwards .
> > >
> > > That is the match.pd patterns are broken for not passing down the
> > > valueizer and checking the results there. I have not looked into
> > > cleaning up the mess either :(.
> >
> > Heh, so I did forget this older advice but in the end I have to agree
> > with myself. The PR hints at a solution with the #define - I'll note
> > that tree_single_nonnegative_warnv_p already has a partial workaround
> > for some issues with checking name_registered_for_update_p.
> >
>
> Ironically you were the one that suggested this fix to me :)
>
> > Partially invalid IL _is_ a problem for SSA walking things, I wonder to
> > what extent we can avoid this here though - short of putting temporary
> > PHI args in there, but those would need to be reliably 'VARYING' in
> > all senses which is difficult - putting in error_mark_node might
> > do the trick?
> >
>
> I could, but it seems odd to have to iterate over every PHI node twice.
> I'll just do that then, it's just so ugly though :(
Well, the alternative is to make you make tree_expr_nonnegative_p
support and honor a valueize callback. The PR has some suggestions
there. Or get rid of the unbound recursion and instead only use
ranger for an SSA def. Or re-implement it as a series of (match ..)
cases (also falling back to ranges for SSA name leafs).
Richard.
> Thanks,
> Tamar.
>
> > Richard.
> >
> > > Thanks,
> > > Andrew
> > >
> > > >
> > > > Thanks,
> > > > Tamar
> > > >
> > > > gcc/ChangeLog:
> > > >
> > > > * gimple-fold.cc (gimple_phi_nonnegative_warnv_p): If the
> > > > arguments
> > > > aren't defined, then return false.
> > > >
> > > > ---
> > > > diff --git a/gcc/gimple-fold.cc b/gcc/gimple-fold.cc
> > > > index
> > edcc04adc08cf15cff36d436caa31c0cfbff8f9a..bed760b9c0d538e2f874f5a1
> > 263a0d2db5486a86 100644
> > > > --- a/gcc/gimple-fold.cc
> > > > +++ b/gcc/gimple-fold.cc
> > > > @@ -11386,7 +11386,9 @@ gimple_phi_nonnegative_warnv_p (gimple
> > *stmt, bool *strict_overflow_p,
> > > > for (unsigned i = 0; i < gimple_phi_num_args (stmt); ++i)
> > > > {
> > > > tree arg = gimple_phi_arg_def (stmt, i);
> > > > - if (!tree_single_nonnegative_warnv_p (arg, strict_overflow_p,
> > > > depth +
> > 1))
> > > > + if (!arg
> > > > + || !tree_single_nonnegative_warnv_p (arg, strict_overflow_p,
> > > > + depth + 1))
> > > > return false;
> > > > }
> > > > return true;
> > > >
> > > >
> > > > --
> > >
> >
> > --
> > Richard Biener <[email protected]>
> > SUSE Software Solutions Germany GmbH,
> > Frankenstrasse 146, 90461 Nuernberg, Germany;
> > GF: Ivo Totev, Andrew McDonald, Werner Knoblich; (HRB 36809, AG
> > Nuernberg)
>
--
Richard Biener <[email protected]>
SUSE Software Solutions Germany GmbH,
Frankenstrasse 146, 90461 Nuernberg, Germany;
GF: Ivo Totev, Andrew McDonald, Werner Knoblich; (HRB 36809, AG Nuernberg)
