RE: [PATCH 1/2]middle-end: fix wide_int_constant_multiple_p when VAL and DIV are 0. [PR114932]

[Tamar Christina <Tamar.Christina@arm.com>]

> -----Original Message-----
> From: Tamar Christina <tamar.christina@arm.com>
> Sent: Monday, July 1, 2024 9:14 PM
> To: gcc-patches@gcc.gnu.org
> Cc: nd <nd@arm.com>; rguenther@suse.de; jlaw@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'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;
> 
> 
> 
> 
> --