Re: [PATCH] MATCH: Maybe expand (T)(A + C1) * C2 and (T)(A + C1) * C2 + C3 [PR109393]

[Richard Biener <rguenther@suse.de>]

On Tue, 23 Apr 2024, Manolis Tsamis wrote:

> The original motivation for this pattern was that the following function does
> not fold to 'return 1':
> 
> int foo(int *a, int j)
> {
>   int k = j - 1;
>   return a[j - 1] == a[k];
> }
> 
> The expression ((unsigned long) (X +- C1) * C2) appears frequently as part of
> address calculations (e.g. arrays). These patterns help fold and simplify more
> expressions.
> 
> 	PR tree-optimization/109393
> 
> gcc/ChangeLog:
> 
> 	* match.pd: Add new patterns for ((T)(A +- CST1)) * CST2 and
> 	  ((T)(A +- CST1)) * CST2 + CST3.
> 
> gcc/testsuite/ChangeLog:
> 
> 	* gcc.dg/pr109393.c: New test.
> 
> Signed-off-by: Manolis Tsamis <manolis.tsamis@vrull.eu>
> ---
> 
>  gcc/match.pd                    | 30 ++++++++++++++++++++++++++++++
>  gcc/testsuite/gcc.dg/pr109393.c | 16 ++++++++++++++++
>  2 files changed, 46 insertions(+)
>  create mode 100644 gcc/testsuite/gcc.dg/pr109393.c
> 
> diff --git a/gcc/match.pd b/gcc/match.pd
> index d401e7503e6..13c828ba70d 100644
> --- a/gcc/match.pd
> +++ b/gcc/match.pd
> @@ -3650,6 +3650,36 @@ DEFINE_INT_AND_FLOAT_ROUND_FN (RINT)
>         (plus (convert @0) (op @2 (convert @1))))))
>  #endif
>  
> +/* ((T)(A + CST1)) * CST2 + CST3
> +     -> ((T)(A) * CST2) + ((T)CST1 * CST2 + CST3)
> +   Where (A + CST1) doesn't need to have a single use.  */
> +#if GIMPLE
> +  (for op (plus minus)
> +   (simplify
> +    (plus (mult (convert:s (op @0 INTEGER_CST@1)) INTEGER_CST@2) INTEGER_CST@3)
> +     (if (TREE_CODE (TREE_TYPE (@0)) == INTEGER_TYPE
> +	  && TREE_CODE (type) == INTEGER_TYPE
> +	  && TYPE_PRECISION (type) > TYPE_PRECISION (TREE_TYPE (@0))
> +	  && TYPE_OVERFLOW_UNDEFINED (TREE_TYPE (@0))
> +	  && !TYPE_OVERFLOW_SANITIZED (TREE_TYPE (@0))
> +	  && TYPE_OVERFLOW_WRAPS (type))
> +       (op (mult @2 (convert @0)) (plus (mult @2 (convert @1)) @3)))))
> +#endif
> +
> +/* ((T)(A + CST1)) * CST2 -> ((T)(A) * CST2) + ((T)CST1 * CST2)  */
> +#if GIMPLE
> +  (for op (plus minus)
> +   (simplify
> +    (mult (convert:s (op:s @0 INTEGER_CST@1)) INTEGER_CST@2)
> +     (if (TREE_CODE (TREE_TYPE (@0)) == INTEGER_TYPE

Please use INTEGRAL_TYPE_P

> +	  && TREE_CODE (type) == INTEGER_TYPE
> +	  && TYPE_PRECISION (type) > TYPE_PRECISION (TREE_TYPE (@0))
> +	  && TYPE_OVERFLOW_UNDEFINED (TREE_TYPE (@0))
> +	  && !TYPE_OVERFLOW_SANITIZED (TREE_TYPE (@0))
> +	  && TYPE_OVERFLOW_WRAPS (type))
> +       (op (mult @2 (convert @0)) (mult @2 (convert @1))))))

(mult @2 (convert @0)) is non-canonical for no good reason if @0
isn't constant - constant should be 2nd, please swap operands here.

> +#endif

The first pattern is an extension of the second, why's the first
necessary at all?  The add of CST3 is unchanged (OK, you seem to
associate here, but that's again a different thing).

I'd say the 2nd pattern is OK with the above changes but the first
looks redundant.

Thanks,
Richard.

> +
>  /* (T)(A) +- (T)(B) -> (T)(A +- B) only when (A +- B) could be simplified
>     to a simple value.  */
>    (for op (plus minus)
> diff --git a/gcc/testsuite/gcc.dg/pr109393.c b/gcc/testsuite/gcc.dg/pr109393.c
> new file mode 100644
> index 00000000000..e9051273672
> --- /dev/null
> +++ b/gcc/testsuite/gcc.dg/pr109393.c
> @@ -0,0 +1,16 @@
> +/* PR tree-optimization/109393 */
> +/* { dg-do compile } */
> +/* { dg-options "-O2 -fdump-tree-optimized" } */
> +/* { dg-final { scan-tree-dump-times "return 1;" 2 "optimized" } } */
> +
> +int foo(int *a, int j)
> +{
> +  int k = j - 1;
> +  return a[j - 1] == a[k];
> +}
> +
> +int bar(int *a, int j)
> +{
> +  int k = j - 1;
> +  return (&a[j + 1] - 2) == &a[k];
> +}
> 

-- 
Richard Biener <rguenther@suse.de>
SUSE Software Solutions Germany GmbH,
Frankenstrasse 146, 90461 Nuernberg, Germany;
GF: Ivo Totev, Andrew McDonald, Werner Knoblich; (HRB 36809, AG Nuernberg)