> -----Original Message-----
> From: Richard Biener <rguent...@suse.de>
> Sent: Tuesday, July 16, 2024 12:47 PM
> To: Tamar Christina <tamar.christ...@arm.com>
> Cc: gcc-patches@gcc.gnu.org; nd <n...@arm.com>; j...@ventanamicro.com
> Subject: Re: [PATCH]middle-end: fix 0 offset creation and folding [PR115936]
>
> On Tue, 16 Jul 2024, Tamar Christina wrote:
>
> > Hi All,
> >
> > As shown in PR115936 SCEV and IVOPTS create an invalidate IV when the IV is
> > a pointer type:
> >
> > ivtmp.39_65 = ivtmp.39_59 + 0B;
> >
> > where the IVs are DI mode and the offset is a pointer.
> > This comes from this weird candidate:
> >
> > Candidate 8:
> > Var befor: ivtmp.39_59
> > Var after: ivtmp.39_65
> > Incr POS: before exit test
> > IV struct:
> > Type: sizetype
> > Base: 0
> > Step: 0B
> > Biv: N
> > Overflowness wrto loop niter: No-overflow
> >
> > This IV was always created just ended up not being used.
> >
> > This is created by SCEV.
> >
> > simple_iv_with_niters in the case where no CHREC is found creates an IV with
> > base == ev, offset == 0;
> >
> > however in this case EV is a POINTER_PLUS_EXPR and so the type is a pointer.
> > it ends up creating an unusable expression.
> >
> > However IVOPTS also has code to refold expression in case the IV is a
> > pointer.
> > For most cases it uses basetype to fold both operand but the 0 case it
> > ommitted
> > it. This leads to us creating a PLUS expression with mismatched types.
> >
> > This fixes that bug as well.
> >
> > Bootstrapped Regtested on aarch64-none-linux-gnu,
> > x86_64-pc-linux-gnu -m32, -m64 and no issues.
> >
> > Ok for master?
> >
> > Thanks,
> > Tamar
> >
> > gcc/ChangeLog:
> >
> > PR tree-optimization/115936
> > * tree-scalar-evolution.cc (simple_iv_with_niters): Use sizetype for
> > pointers.
> > * tree-ssa-loop-ivopts.cc (add_iv_candidate_for_use): Use same type for
> > both operands.
> >
> > ---
> > diff --git a/gcc/tree-scalar-evolution.cc b/gcc/tree-scalar-evolution.cc
> > index
> 5aa95a2497a317f9b43408ce78a2d50c20151314..abb2bad777375889d6c980b
> 54d60699672fd5742 100644
> > --- a/gcc/tree-scalar-evolution.cc
> > +++ b/gcc/tree-scalar-evolution.cc
> > @@ -3243,7 +3243,11 @@ simple_iv_with_niters (class loop *wrto_loop, class
> loop *use_loop,
> > if (tree_does_not_contain_chrecs (ev))
> > {
> > iv->base = ev;
> > - iv->step = build_int_cst (TREE_TYPE (ev), 0);
> > + tree ev_type = TREE_TYPE (ev);
> > + if (POINTER_TYPE_P (ev_type))
> > + ev_type = sizetype;
> > +
> > + iv->step = build_int_cst (ev_type, 0);
> > iv->no_overflow = true;
> > return true;
> > }
>
> This hunk is OK.
>
> > diff --git a/gcc/tree-ssa-loop-ivopts.cc b/gcc/tree-ssa-loop-ivopts.cc
> > index
> c3218a3e8eedbb8d0a7f14c01eeb069cb6024c29..fe130541526e74fb80fee633f
> 6c96b41437aa1c1 100644
> > --- a/gcc/tree-ssa-loop-ivopts.cc
> > +++ b/gcc/tree-ssa-loop-ivopts.cc
> > @@ -3529,7 +3529,8 @@ add_iv_candidate_for_use (struct ivopts_data *data,
> struct iv_use *use)
> > basetype = TREE_TYPE (iv->base);
> > if (POINTER_TYPE_P (basetype))
> > basetype = sizetype;
> > - record_common_cand (data, build_int_cst (basetype, 0), iv->step, use);
> > + record_common_cand (data, build_int_cst (basetype, 0),
> > + fold_convert (basetype, iv->step), use);
>
> But this looks redundant? iv->step should already be sizetype for a
> pointer base?
Yeah, on a correctly formed candidate. I thought it would be a good idea to be
a bit more
resilient in the cases where we're changing the base anyway. But I equally
understand that
we'd want to know when a candidate is malformed.
So I'll go with only the first hunk.
Thanks,
Tamar
>
> Thanks,
> Richard.
>
> > /* Compare the cost of an address with an unscaled index with the cost of
> > an address with a scaled index and add candidate if useful. */
> >
> >
> >
> >
> >
>
> --
> Richard Biener <rguent...@suse.de>
> SUSE Software Solutions Germany GmbH,
> Frankenstrasse 146, 90461 Nuernberg, Germany;
> GF: Ivo Totev, Andrew McDonald, Werner Knoblich; (HRB 36809, AG Nuernberg)
