On Tue, May 27, 2025 at 5:14 AM Richard Biener
<richard.guent...@gmail.com> wrote:
>
> On Tue, May 27, 2025 at 5:02 AM Andrew Pinski <quic_apin...@quicinc.com>
> wrote:
> >
> > This was noticed in the review of copy propagation for aggregates
> > patch, instead of checking for a NULL or a non-ssa name of vuse,
> > we should instead check if it the vuse is a default name and stop
> > then.
> >
> > Bootstrapped and tested on x86_64-linux-gnu.
> >
> > gcc/ChangeLog:
> >
> > * tree-ssa-forwprop.cc (optimize_memcpy_to_memset): Change check
> > from NULL/non-ssa name to default name.
> >
> > Signed-off-by: Andrew Pinski <quic_apin...@quicinc.com>
> > ---
> > gcc/tree-ssa-forwprop.cc | 3 ++-
> > 1 file changed, 2 insertions(+), 1 deletion(-)
> >
> > diff --git a/gcc/tree-ssa-forwprop.cc b/gcc/tree-ssa-forwprop.cc
> > index 4c048a9a298..e457a69ed48 100644
> > --- a/gcc/tree-ssa-forwprop.cc
> > +++ b/gcc/tree-ssa-forwprop.cc
> > @@ -1226,7 +1226,8 @@ optimize_memcpy_to_memset (gimple_stmt_iterator
> > *gsip, tree dest, tree src, tree
> > gimple *defstmt;
> > unsigned limit = param_sccvn_max_alias_queries_per_access;
> > do {
> > - if (vuse == NULL || TREE_CODE (vuse) != SSA_NAME)
> > + /* If the vuse is the default definition, then there is no stores
> > beforhand. */
> > + if (SSA_NAME_IS_DEFAULT_DEF (vuse))
>
> Since forwprop does update_ssa in the end I was wondering whether any
> bare non-SSA VUSE/VDEFs sneak in - for this the != SSA_NAME check
> would be useful. On a GIMPLE stmt gimple_vuse () will return NULL
> when it's not a load or store (or with a novops call), as you are using
> gimple_store_p/gimple_assign_load_p there might be a disconnect
> between those predicates and the presence of a vuse (I hope not, but ...)
>
> The patch looks OK to me, the comments above apply to the copy propagation
> case.
The copy prop case should be ok too since the vuse/vdef on the
statement does not change when doing the prop; only the rhs of the
statement. There is no inserting of a statement. This is unless we
remove the statement and then unlink_stmt_vdef will prop the vuse into
the vdef of the statement which we are removing.
I did test the copy prop using just SSA_NAME_IS_DEFAULT_DEF and there
were no regressions there either.
When optimize_memcpy_to_memset was part of fold_stmt, a NULL vuse
and/or a non-SSA vuse was common due to running before ssa. This was
why there was a check for non-SSA.
I am not sure why there was a check for NULLness was there when it was
part of fold-all-builtins though.
On a side note I think many passes have TODO_update_ssa on them when
they already keep the ssa up to date now. I wonder if most of that
dates from the days of VMUST_DEF/VMAY_DEF and multiple names on them
rather than one virtual name.
Thanks,
Andrew
>
> Thanks,
> Richard.
>
> > return false;
> > defstmt = SSA_NAME_DEF_STMT (vuse);
> > if (is_a <gphi*>(defstmt))
> > --
> > 2.43.0
> >
