Hi,
On 2020/2/24 4:16 PM, Marc Glisse wrote:
On Mon, 24 Feb 2020, Li Jia He wrote:
Hi,
On 2020/2/22 11:12 PM, Marc Glisse wrote:
On Tue, 18 Feb 2020, Li Jia He wrote:
Also the pattern doing the standalone transform does
/* Optimize TRUNC_MOD_EXPR by a power of two into a BIT_AND_EXPR,
i.e. "X % C" into "X & (C - 1)", if X and C are positive.
Also optimize A % (C << N) where C is a power of 2,
to A & ((C << N) - 1). */
(match (power_of_two_cand @1)
INTEGER_CST@1)
(match (power_of_two_cand @1)
(lshift INTEGER_CST@1 @2))
(for mod (trunc_mod floor_mod)
(simplify
(mod @0 (convert?@3 (power_of_two_cand@1 @2)))
(if ((TYPE_UNSIGNED (type)
|| tree_expr_nonnegative_p (@0))
&& tree_nop_conversion_p (type, TREE_TYPE (@3))
&& integer_pow2p (@2) && tree_int_cst_sgn (@2) > 0)
(bit_and @0 (convert (minus @1 { build_int_cst (TREE_TYPE (@1),
1);
}))))))
so it also checks whether @2 is not negative, the new pattern lacks
this and could make use of power_of_two_cand as well (in fact I'd
place it next to the pattern above.
Thank you for your suggestions. I have modified the code according
to your
suggestions. But power_of_two_cand is not used, it will cause
pr70354-2.c
failed ((0x1234ULL << (i % 54)) will transfer to (0x1234ULL << (i &
54))).
How exactly did you use power_of_two_cand? As shown in the
transformation above, it combines with integer_pow2p, it doesn't
replace it.
I used power_of_two_cand as follows：
diff --git a/gcc/match.pd b/gcc/match.pd
index 73834c25593..a90e93d8af0 100644
--- a/gcc/match.pd
+++ b/gcc/match.pd
@@ -598,12 +598,19 @@ DEFINE_INT_AND_FLOAT_ROUND_FN (RINT)
/* Optimize TRUNC_MOD_EXPR by a power of two into a BIT_AND_EXPR,
i.e. "X % C" into "X & (C - 1)", if X and C are positive.
Also optimize A % (C << N) where C is a power of 2,
- to A & ((C << N) - 1). */
+ to A & ((C << N) - 1). And optimize "A shift (N % C)" where C
+ is a power of 2 and shift operation included "<<" and ">>",
+ to "A shift (N & (C - 1))". */
(match (power_of_two_cand @1)
INTEGER_CST@1)
(match (power_of_two_cand @1)
(lshift INTEGER_CST@1 @2))
(for mod (trunc_mod floor_mod)
+ (for shift (lshift rshift)
+ (simplify
+ (shift @0 (mod @1 (power_of_two_cand @2)))
+ (if (tree_expr_nonnegative_p (@2))
+ (shift @0 (bit_and @1 (minus @2 { build_int_cst (TREE_TYPE (@2),
1); }))))))
(simplify
(mod @0 (convert?@3 (power_of_two_cand@1 @2)))
(if ((TYPE_UNSIGNED (type)
I found that there will be a generated tree_power_of_two_cand function in
generic-match.c and gimple_power_of_two_cand function in gimple-match.c.
bool
tree_power_of_two_cand (tree t, tree *res_ops)
{
const tree type = TREE_TYPE (t);
if (TREE_SIDE_EFFECTS (t)) return false;
switch (TREE_CODE (t))
{
case INTEGER_CST:
{
{
/* #line 604 "../../gcc-mirror/gcc/match.pd" */
tree captures[1] ATTRIBUTE_UNUSED = { t };
if (__builtin_expect (dump_file && (dump_flags &
TDF_FOLDING), 0)) fprintf (dump_file, "Matching expression %s:%d,
%s:%d\n", "match.pd", 604, __FILE__, __LINE__);
res_ops[0] = captures[0];
return true;
}
break;
}
case LSHIFT_EXPR:
{
tree _p0 = TREE_OPERAND (t, 0);
tree _p1 = TREE_OPERAND (t, 1);
switch (TREE_CODE (_p0))
{
case INTEGER_CST:
{
{
/* #line 606 "../../gcc-mirror/gcc/match.pd" */
tree captures[2] ATTRIBUTE_UNUSED = { _p0, _p1 };
if (__builtin_expect (dump_file && (dump_flags &
TDF_FOLDING), 0)) fprintf (dump_file, "Matching expression %s:%d,
%s:%d\n", "match.pd", 606, __FILE__, __LINE__);
res_ops[0] = captures[0];
return true;
}
break;
}
default:;
}
break;
}
default:;
}
return false;
}
In the tree_power_of_two_cand function, we can see that if t is an
INTEGER_CST, it will be captured directly, so using power_of_two_cand
may not be appropriate here.
Please look at the one transformation that already uses
power_of_two_cand. It matches (power_of_two_cand@1 @2), then tests
integer_pow2p (@2) && tree_int_cst_sgn (@2) > 0, and uses @1 in the
output. In some sense, tree_power_of_two_cand prepares the argument for
a call to integer_pow2p, it doesn't replace it (although we could move
the extra tests into power_of_two_cand if we expect all users will need
exactly the same).
Thank you for your guidance. I have made some changes based on my
understanding. Would you like to see it again? Thank you in advance.
The code is in the attachment.
--
BR,
Lijia He
diff --git a/gcc/match.pd b/gcc/match.pd
index 73834c25593..9e7619c5be8 100644
--- a/gcc/match.pd
+++ b/gcc/match.pd
@@ -598,12 +598,21 @@ DEFINE_INT_AND_FLOAT_ROUND_FN (RINT)
/* Optimize TRUNC_MOD_EXPR by a power of two into a BIT_AND_EXPR,
i.e. "X % C" into "X & (C - 1)", if X and C are positive.
Also optimize A % (C << N) where C is a power of 2,
- to A & ((C << N) - 1). */
+ to A & ((C << N) - 1). And optimize "A shift (B % C)" where C
+ is a power of 2, shift operation included "<<" and ">>" and assume
+ (B % C) will not be negative as shifts negative values would be UB,
+ to "A shift (B & (C - 1))". */
(match (power_of_two_cand @1)
INTEGER_CST@1)
(match (power_of_two_cand @1)
(lshift INTEGER_CST@1 @2))
(for mod (trunc_mod floor_mod)
+ (for shift (lshift rshift)
+ (simplify
+ (shift @0 (mod @1 (power_of_two_cand@2 @3)))
+ (if (integer_pow2p (@3) && tree_int_cst_sgn (@3) > 0)
+ (shift @0 (bit_and @1 (minus @2 { build_int_cst (TREE_TYPE (@2),
+ 1); }))))))
(simplify
(mod @0 (convert?@3 (power_of_two_cand@1 @2)))
(if ((TYPE_UNSIGNED (type)
diff --git a/gcc/testsuite/gcc.dg/pr66552.c b/gcc/testsuite/gcc.dg/pr66552.c
new file mode 100644
index 00000000000..7583c9ad25a
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/pr66552.c
@@ -0,0 +1,14 @@
+/* { dg-do compile } */
+/* { dg-options "-O2 -fdump-tree-lower" } */
+
+unsigned a(unsigned x, int n)
+{
+ return x >> (n % 32);
+}
+
+unsigned b(unsigned x, int n)
+{
+ return x << (n % 32);
+}
+
+/* { dg-final { scan-tree-dump-not " % " "lower" } } */
