Re: [PATCH] match.pd: Simplify (t * u) / v -> t * (u / v) [PR112994]

[Richard Biener <rguenther@suse.de>]

> Am 14.12.2023 um 08:35 schrieb Jakub Jelinek <jakub@redhat.com>:
> 
> Hi!
> 
> The following testcase is optimized just on GENERIC (using
>      strict_overflow_p = false;
>      if (TREE_CODE (arg1) == INTEGER_CST
>          && (tem = extract_muldiv (op0, arg1, code, NULL_TREE,
>                                    &strict_overflow_p)) != 0)
>        {
>          if (strict_overflow_p)
>            fold_overflow_warning (("assuming signed overflow does not occur "
>                                    "when simplifying division"),
>                                   WARN_STRICT_OVERFLOW_MISC);
>          return fold_convert_loc (loc, type, tem);
>        }
> ) but not on GIMPLE.
> 
> The following included patch is what I've bootstrapped/regtested
> for it on x86_64-linux and i686-linux, unfortunately it regressed
> +FAIL: gcc.dg/Wstrict-overflow-3.c correct warning (test for warnings, line 12)
> test, we are indeed assuming that signed overflow does not occur
> when simplifying division in there.
> 
> The attached version of the patch (which provides the simplification only
> for GIMPLE) fixes that, but I haven't bootstrapped/regtested it yet.
> And/or we could add the
>            fold_overflow_warning (("assuming signed overflow does not occur "
>                                    "when simplifying division"),
>                                   WARN_STRICT_OVERFLOW_MISC);
> call into the simplification, but in that case IMHO it should go into
> the (t * u) / u -> t simplification as well, there we assume the exact
> same thing (of course, in both cases only in the spots where we don't
> verify it through ranger that it never overflows).
> 
> Guarding the whole simplification to GIMPLE only IMHO makes sense because
> the above mentioned folding does it for GENERIC (and extract_muldiv even
> handles far more cases, dunno how many from that we should be doing on
> GIMPLE in match.pd and what could be done elsewhere; e.g. extract_muldiv
> can handle (x * 16 + y * 32) / 8 -> x * 2 + y * 4 etc.).
> 
> Dunno about the fold_overflow_warning, I always have doubts about why
> such a warning is useful to users.
> 
> Ok for trunk (and which version)?

I couldn’t spot the difference… OK for either
Version (and no, calling fold_overflow_warning looks wrong)

Richard 

> 2023-12-14  Jakub Jelinek  <jakub@redhat.com>
> 
>    PR tree-optimization/112994
>    * match.pd ((t * 2) / 2 -> t): Adjust comment to use u instead of 2.
>    Punt without range checks if TYPE_OVERFLOW_SANITIZED.
>    ((t * u) / v -> t * (u / v)): New simplification.
> 
>    * gcc.dg/tree-ssa/pr112994-1.c: New test.
> 
> --- gcc/match.pd.jj    2023-12-13 11:21:15.852158970 +0100
> +++ gcc/match.pd    2023-12-13 19:10:26.448927327 +0100
> @@ -930,12 +930,12 @@ DEFINE_INT_AND_FLOAT_ROUND_FN (RINT)
>  (if (INTEGRAL_TYPE_P (TREE_TYPE (@0)) && TYPE_UNSIGNED (TREE_TYPE (@0)))
>   (bit_and @0 (negate @1))))
> 
> -/* Simplify (t * 2) / 2) -> t.  */
> (for div (trunc_div ceil_div floor_div round_div exact_div)
> + /* Simplify (t * u) / u -> t.  */
>  (simplify
>   (div (mult:c @0 @1) @1)
>   (if (ANY_INTEGRAL_TYPE_P (type))
> -   (if (TYPE_OVERFLOW_UNDEFINED (type))
> +   (if (TYPE_OVERFLOW_UNDEFINED (type) && !TYPE_OVERFLOW_SANITIZED (type))
>     @0
> #if GIMPLE
>     (with {value_range vr0, vr1;}
> @@ -945,6 +945,21 @@ DEFINE_INT_AND_FLOAT_ROUND_FN (RINT)
>      && range_op_handler (MULT_EXPR).overflow_free_p (vr0, vr1))
>       @0))
> #endif
> +   )))
> + /* Simplify (t * u) / v -> t * (u / v) if u is multiple of v.  */
> + (simplify
> +  (div (mult @0 INTEGER_CST@1) INTEGER_CST@2)
> +  (if (INTEGRAL_TYPE_P (type)
> +       && wi::multiple_of_p (wi::to_widest (@1), wi::to_widest (@2), SIGNED))
> +   (if (TYPE_OVERFLOW_UNDEFINED (type) && !TYPE_OVERFLOW_SANITIZED (type))
> +    (mult @0 (div! @1 @2))
> +#if GIMPLE
> +    (with {value_range vr0, vr1;}
> +     (if (get_range_query (cfun)->range_of_expr (vr0, @0)
> +      && get_range_query (cfun)->range_of_expr (vr1, @1)
> +      && range_op_handler (MULT_EXPR).overflow_free_p (vr0, vr1))
> +      (mult @0 (div! @1 @2))))
> +#endif
>    ))))
> 
> #if GIMPLE
> --- gcc/testsuite/gcc.dg/tree-ssa/pr112994-1.c.jj    2023-12-13 16:58:25.757663610 +0100
> +++ gcc/testsuite/gcc.dg/tree-ssa/pr112994-1.c    2023-12-13 16:43:16.413152969 +0100
> @@ -0,0 +1,13 @@
> +/* PR tree-optimization/112994 */
> +/* { dg-do compile } */
> +/* { dg-options "-O2 -fdump-tree-optimized" } */
> +/* { dg-final { scan-tree-dump-not " / \\\[2389-\\\]" "optimized" } } */
> +
> +int f1 (int x) { return (x * 4) / 2; }
> +int f2 (int x) { return (x * 56) / 8; }
> +int f3 (int x) { return (x * 56) / -8; }
> +int f4 (int x) { int y = x * 4; return y / 2; }
> +int f5 (int x) { int y = x * 56; return y / 8; }
> +int f6 (int x) { int y = x * 56; return y / -8; }
> +unsigned f7 (unsigned x) { if (x > ~0U / 6) __builtin_unreachable (); unsigned y = x * 6; return y / 3; }
> +unsigned f8 (unsigned x) { if (x > ~0U / 63) __builtin_unreachable (); unsigned y = x * 63; return y / 9; }
> 
>    Jakub
> <Q142>