On Mon, 1 Jul 2024, Tamar Christina wrote:
> > -----Original Message-----
> > From: Tamar Christina <tamar.christ...@arm.com>
> > Sent: Monday, July 1, 2024 9:14 PM
> > To: gcc-patches@gcc.gnu.org
> > Cc: nd <n...@arm.com>; rguent...@suse.de; j...@ventanamicro.com
> > Subject: [PATCH 1/2]middle-end: fix wide_int_constant_multiple_p when VAL
> > and
> > DIV are 0. [PR114932]
> >
> > Hi All,
> >
> > wide_int_constant_multiple_p tries to check if for two tree expressions a
> > and b
> > that there is a multiplier which makes a == b * c.
> >
> > This code however seems to think that there's no c where a=0 and b=0 are
> > equal
> > which is of course wrong.
> >
> > This fixes it and also fixes the comment.
> >
> > 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/114932
> > * tree-affine.cc (wide_int_constant_multiple_p): Support 0 and 0 being
> > multiples.
> >
> > ---
> > diff --git a/gcc/tree-affine.cc b/gcc/tree-affine.cc
> > index
> > d6309c4390362b680f0aa97a41fac3281ade66fd..bfea0fe826a6affa0ace154e3ca
> > 38c9ef632fcba 100644
> > --- a/gcc/tree-affine.cc
> > +++ b/gcc/tree-affine.cc
> > @@ -880,11 +880,10 @@ free_affine_expand_cache (hash_map<tree,
> > name_expansion *> **cache)
> > *cache = NULL;
> > }
> >
> > -/* If VAL != CST * DIV for any constant CST, returns false.
> > - Otherwise, if *MULT_SET is true, additionally compares CST and MULT,
> > - and if they are different, returns false. Finally, if neither of these
> > - two cases occur, true is returned, and CST is stored to MULT and
> > MULT_SET
> > - is set to true. */
> > +/* If VAL == CST * DIV for any constant CST, returns true.
> > + and if *MULT_SET is true, additionally compares CST and MULT
> > + and if they are different, returns false. If true is returned, CST is
> > + stored to MULT and MULT_SET is set to true. */
> >
> > static bool
> > wide_int_constant_multiple_p (const poly_widest_int &val,
> > @@ -895,6 +894,12 @@ wide_int_constant_multiple_p (const poly_widest_int
> > &val,
> >
> > if (known_eq (val, 0))
> > {
> > + if (maybe_eq (div, 0))
> > + {
> > + *mult = 1;
> > + return true;
> > + }
> > +
>
> Note, I also tested known_eq here, and also no regression on what I can test.
> I picked maybe_eq since that's what the lines after this one tests.
I think the maybe_eq (div, 0) is because otherwise multiple_p might
crash? I'm not sure if there's a difference between
maybe_eq (x, 0) and known_eq (x, 0) though - how does a maybe_eq
POLY_INT look like that's not known_eq?
> I'm not sure I fully understand why one tests known and the other maybe. It
> seems to me
> that both should test known. But I tested both so which ever one is felt to
> be more correct
> I can commit If ok.
>
> Thanks,
> Tamar
>
> > if (*mult_set && maybe_ne (*mult, 0))
> > return false;
> > *mult_set = true;
> >
> >
> >
> >
> > --
>
--
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)
