public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
* [PATCH] match.pd: Simplify (t * u) / v -> t * (u / v) [PR112994]
@ 2023-12-14  7:34 Jakub Jelinek
  2023-12-14 10:41 ` Richard Biener
  0 siblings, 1 reply; 2+ messages in thread
From: Jakub Jelinek @ 2023-12-14  7:34 UTC (permalink / raw)
  To: Richard Biener; +Cc: gcc-patches

[-- Attachment #1: Type: text/plain, Size: 4642 bytes --]

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)?

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

[-- Attachment #2: Q142 --]
[-- Type: text/plain, Size: 2635 bytes --]

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-14 08:22:43.090755645 +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,7 +945,23 @@ DEFINE_INT_AND_FLOAT_ROUND_FN (RINT)
 	  && range_op_handler (MULT_EXPR).overflow_free_p (vr0, vr1))
       @0))
 #endif
-   ))))
+   )))
+#if GIMPLE
+ /* 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))
+    (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
 (for div (trunc_div exact_div)
--- 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; }

^ permalink raw reply	[flat|nested] 2+ messages in thread

* Re: [PATCH] match.pd: Simplify (t * u) / v -> t * (u / v) [PR112994]
  2023-12-14  7:34 [PATCH] match.pd: Simplify (t * u) / v -> t * (u / v) [PR112994] Jakub Jelinek
@ 2023-12-14 10:41 ` Richard Biener
  0 siblings, 0 replies; 2+ messages in thread
From: Richard Biener @ 2023-12-14 10:41 UTC (permalink / raw)
  To: Jakub Jelinek; +Cc: gcc-patches



> 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>

^ permalink raw reply	[flat|nested] 2+ messages in thread

end of thread, other threads:[~2023-12-14 10:41 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-12-14  7:34 [PATCH] match.pd: Simplify (t * u) / v -> t * (u / v) [PR112994] Jakub Jelinek
2023-12-14 10:41 ` Richard Biener

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).