Re: [PATCH] match.pd: Improve (A / (1 << B)) -> (A >> B) optimization [PR96930]

2021-01-05 Thread Richard Biener
On Tue, 5 Jan 2021, Jakub Jelinek wrote:

> Hi!
> 
> The following patch improves the A / (1 << B) -> A >> B simplification,
> as seen in the testcase, if there is unnecessary widening for the division,
> we just optimize it into a shift on the widened type, but if the lshift
> is widened too, there is no reason to do that, we can just shift it in the
> original type and convert after.  The tree_nonzero_bits & wi::mask check
> already ensures it is fine even for signed values.
> 
> I've split the vr-values optimization into a separate patch as it causes
> a small regression on two testcases, but this patch fixes what has been
> reported in the PR alone.
> 
> Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?

OK.

Thanks,
Richard.

> 2021-01-05  Jakub Jelinek  
> 
>   PR tree-optimization/96930
>   * match.pd ((A / (1 << B)) -> (A >> B)): If A is extended
>   from narrower value which has the same type as 1 << B, perform
>   the right shift on the narrower value followed by extension.
> 
>   * g++.dg/tree-ssa/pr96930.C: New test.
> 
> --- gcc/match.pd.jj   2021-01-04 10:37:06.0 +0100
> +++ gcc/match.pd  2021-01-05 10:27:56.653791400 +0100
> @@ -321,7 +321,7 @@ DEFINE_INT_AND_FLOAT_ROUND_FN (RINT)
> (unsigned long long) (1 << 31) is -2147483648ULL, not 2147483648ULL,
> so it is valid only if A >> 31 is zero.  */
>  (simplify
> - (trunc_div @0 (convert? (lshift integer_onep@1 @2)))
> + (trunc_div (convert?@0 @3) (convert2? (lshift integer_onep@1 @2)))
>   (if ((TYPE_UNSIGNED (type) || tree_expr_nonnegative_p (@0))
>&& (!VECTOR_TYPE_P (type)
> || target_supports_op_p (type, RSHIFT_EXPR, optab_vector)
> @@ -336,7 +336,11 @@ DEFINE_INT_AND_FLOAT_ROUND_FN (RINT)
> & wi::mask (element_precision (TREE_TYPE (@1)) - 1,
> true,
> element_precision (type))) == 0)
> -  (rshift @0 @2)))
> +   (if (!VECTOR_TYPE_P (type)
> + && useless_type_conversion_p (TREE_TYPE (@3), TREE_TYPE (@1))
> + && element_precision (TREE_TYPE (@3)) < element_precision (type))
> +(convert (rshift @3 @2))
> +(rshift @0 @2
>  
>  /* Preserve explicit divisions by 0: the C++ front-end wants to detect
> undefined behavior in constexpr evaluation, and assuming that the division
> --- gcc/testsuite/g++.dg/tree-ssa/pr96930.C.jj2021-01-04 
> 14:18:15.513100038 +0100
> +++ gcc/testsuite/g++.dg/tree-ssa/pr96930.C   2021-01-04 14:25:35.512148709 
> +0100
> @@ -0,0 +1,10 @@
> +// PR tree-optimization/96930
> +// { dg-do compile }
> +// { dg-options "-O2 -fdump-tree-optimized" }
> +// { dg-final { scan-tree-dump " = a_\[0-9]\\\(D\\\) >> b_\[0-9]\\\(D\\\);" 
> "optimized" } }
> +
> +unsigned
> +foo (unsigned a, unsigned b)
> +{
> +  return a / (unsigned long long) (1U << b);
> +}
> 
>   Jakub
> 
> 

-- 
Richard Biener 
SUSE Software Solutions Germany GmbH, Maxfeldstrasse 5, 90409 Nuernberg,
Germany; GF: Felix Imendörffer; HRB 36809 (AG Nuernberg)


[PATCH] match.pd: Improve (A / (1 << B)) -> (A >> B) optimization [PR96930]

2021-01-05 Thread Jakub Jelinek via Gcc-patches
Hi!

The following patch improves the A / (1 << B) -> A >> B simplification,
as seen in the testcase, if there is unnecessary widening for the division,
we just optimize it into a shift on the widened type, but if the lshift
is widened too, there is no reason to do that, we can just shift it in the
original type and convert after.  The tree_nonzero_bits & wi::mask check
already ensures it is fine even for signed values.

I've split the vr-values optimization into a separate patch as it causes
a small regression on two testcases, but this patch fixes what has been
reported in the PR alone.

Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?

2021-01-05  Jakub Jelinek  

PR tree-optimization/96930
* match.pd ((A / (1 << B)) -> (A >> B)): If A is extended
from narrower value which has the same type as 1 << B, perform
the right shift on the narrower value followed by extension.

* g++.dg/tree-ssa/pr96930.C: New test.

--- gcc/match.pd.jj 2021-01-04 10:37:06.0 +0100
+++ gcc/match.pd2021-01-05 10:27:56.653791400 +0100
@@ -321,7 +321,7 @@ DEFINE_INT_AND_FLOAT_ROUND_FN (RINT)
(unsigned long long) (1 << 31) is -2147483648ULL, not 2147483648ULL,
so it is valid only if A >> 31 is zero.  */
 (simplify
- (trunc_div @0 (convert? (lshift integer_onep@1 @2)))
+ (trunc_div (convert?@0 @3) (convert2? (lshift integer_onep@1 @2)))
  (if ((TYPE_UNSIGNED (type) || tree_expr_nonnegative_p (@0))
   && (!VECTOR_TYPE_P (type)
  || target_supports_op_p (type, RSHIFT_EXPR, optab_vector)
@@ -336,7 +336,11 @@ DEFINE_INT_AND_FLOAT_ROUND_FN (RINT)
  & wi::mask (element_precision (TREE_TYPE (@1)) - 1,
  true,
  element_precision (type))) == 0)
-  (rshift @0 @2)))
+   (if (!VECTOR_TYPE_P (type)
+   && useless_type_conversion_p (TREE_TYPE (@3), TREE_TYPE (@1))
+   && element_precision (TREE_TYPE (@3)) < element_precision (type))
+(convert (rshift @3 @2))
+(rshift @0 @2
 
 /* Preserve explicit divisions by 0: the C++ front-end wants to detect
undefined behavior in constexpr evaluation, and assuming that the division
--- gcc/testsuite/g++.dg/tree-ssa/pr96930.C.jj  2021-01-04 14:18:15.513100038 
+0100
+++ gcc/testsuite/g++.dg/tree-ssa/pr96930.C 2021-01-04 14:25:35.512148709 
+0100
@@ -0,0 +1,10 @@
+// PR tree-optimization/96930
+// { dg-do compile }
+// { dg-options "-O2 -fdump-tree-optimized" }
+// { dg-final { scan-tree-dump " = a_\[0-9]\\\(D\\\) >> b_\[0-9]\\\(D\\\);" 
"optimized" } }
+
+unsigned
+foo (unsigned a, unsigned b)
+{
+  return a / (unsigned long long) (1U << b);
+}

Jakub