* [PATCH 1/2] Move `~X & X` and `~X | X` over to use bitwise_inverted_equal_p
@ 2023-07-31 17:46 Andrew Pinski
2023-07-31 17:46 ` [PATCH 2/2] Slightly improve bitwise_inverted_equal_p comparisons Andrew Pinski
2023-08-02 8:04 ` [PATCH 1/2] Move `~X & X` and `~X | X` over to use bitwise_inverted_equal_p Richard Biener
0 siblings, 2 replies; 6+ messages in thread
From: Andrew Pinski @ 2023-07-31 17:46 UTC (permalink / raw)
To: gcc-patches; +Cc: Andrew Pinski
This is a simple patch to move these 2 patterns over to use
bitwise_inverted_equal_p. It also allows us to remove 2 other patterns
which were used on comparisons as they are now handled by
the original pattern.
OK? Bootstrapped and tested on x86_64-linux-gnu with no regressions.
gcc/ChangeLog:
* match.pd (`~X & X`, `~X | X`): Move over to
use bitwise_inverted_equal_p, removing :c as bitwise_inverted_equal_p
handles that already.
Remove range test simplifications to true/false as they
are now handled by these patterns.
---
gcc/match.pd | 28 ++++++----------------------
1 file changed, 6 insertions(+), 22 deletions(-)
diff --git a/gcc/match.pd b/gcc/match.pd
index 74f0a84f31d..7d030262698 100644
--- a/gcc/match.pd
+++ b/gcc/match.pd
@@ -1157,8 +1157,9 @@ DEFINE_INT_AND_FLOAT_ROUND_FN (RINT)
/* Simplify ~X & X as zero. */
(simplify
- (bit_and:c (convert? @0) (convert? (bit_not @0)))
- { build_zero_cst (type); })
+ (bit_and (convert? @0) (convert? @1))
+ (if (bitwise_inverted_equal_p (@0, @1))
+ { build_zero_cst (type); }))
/* PR71636: Transform x & ((1U << b) - 1) -> x & ~(~0U << b); */
(simplify
@@ -1395,8 +1396,9 @@ DEFINE_INT_AND_FLOAT_ROUND_FN (RINT)
/* ~x ^ x -> -1 */
(for op (bit_ior bit_xor)
(simplify
- (op:c (convert? @0) (convert? (bit_not @0)))
- (convert { build_all_ones_cst (TREE_TYPE (@0)); })))
+ (op (convert? @0) (convert? @1))
+ (if (bitwise_inverted_equal_p (@0, @1))
+ (convert { build_all_ones_cst (TREE_TYPE (@0)); }))))
/* x ^ x -> 0 */
(simplify
@@ -5994,24 +5996,6 @@ DEFINE_INT_AND_FLOAT_ROUND_FN (RINT)
(bit_and:c (ordered @0 @0) (ordered:c@2 @0 @1))
@2)
-/* Simple range test simplifications. */
-/* A < B || A >= B -> true. */
-(for test1 (lt le le le ne ge)
- test2 (ge gt ge ne eq ne)
- (simplify
- (bit_ior:c (test1 @0 @1) (test2 @0 @1))
- (if (INTEGRAL_TYPE_P (TREE_TYPE (@0))
- || VECTOR_INTEGER_TYPE_P (TREE_TYPE (@0)))
- { constant_boolean_node (true, type); })))
-/* A < B && A >= B -> false. */
-(for test1 (lt lt lt le ne eq)
- test2 (ge gt eq gt eq gt)
- (simplify
- (bit_and:c (test1 @0 @1) (test2 @0 @1))
- (if (INTEGRAL_TYPE_P (TREE_TYPE (@0))
- || VECTOR_INTEGER_TYPE_P (TREE_TYPE (@0)))
- { constant_boolean_node (false, type); })))
-
/* A & (2**N - 1) <= 2**K - 1 -> A & (2**N - 2**K) == 0
A & (2**N - 1) > 2**K - 1 -> A & (2**N - 2**K) != 0
--
2.31.1
^ permalink raw reply [flat|nested] 6+ messages in thread* [PATCH 2/2] Slightly improve bitwise_inverted_equal_p comparisons
2023-07-31 17:46 [PATCH 1/2] Move `~X & X` and `~X | X` over to use bitwise_inverted_equal_p Andrew Pinski
@ 2023-07-31 17:46 ` Andrew Pinski
2023-08-02 8:04 ` Richard Biener
2023-08-02 8:04 ` [PATCH 1/2] Move `~X & X` and `~X | X` over to use bitwise_inverted_equal_p Richard Biener
1 sibling, 1 reply; 6+ messages in thread
From: Andrew Pinski @ 2023-07-31 17:46 UTC (permalink / raw)
To: gcc-patches; +Cc: Andrew Pinski
This slighly improves bitwise_inverted_equal_p
for comparisons. Instead of just comparing the
comparisons operands also valueize them.
This will allow ccp and others to match the 2 comparisons
without an extra pass happening.
OK? Bootstrapped and tested on x86_64-linux-gnu.
gcc/ChangeLog:
* gimple-match-head.cc (gimple_bitwise_inverted_equal_p): Valueize
the comparison operands before comparing them.
---
gcc/gimple-match-head.cc | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/gcc/gimple-match-head.cc b/gcc/gimple-match-head.cc
index 0265e55be93..b1e96304d7c 100644
--- a/gcc/gimple-match-head.cc
+++ b/gcc/gimple-match-head.cc
@@ -319,12 +319,12 @@ gimple_bitwise_inverted_equal_p (tree expr1, tree expr2, tree (*valueize) (tree)
&& TREE_CODE_CLASS (gimple_assign_rhs_code (a1)) == tcc_comparison
&& TREE_CODE_CLASS (gimple_assign_rhs_code (a2)) == tcc_comparison)
{
- tree op10 = gimple_assign_rhs1 (a1);
- tree op20 = gimple_assign_rhs1 (a2);
+ tree op10 = do_valueize (valueize, gimple_assign_rhs1 (a1));
+ tree op20 = do_valueize (valueize, gimple_assign_rhs1 (a2));
if (!operand_equal_p (op10, op20))
return false;
- tree op11 = gimple_assign_rhs2 (a1);
- tree op21 = gimple_assign_rhs2 (a2);
+ tree op11 = do_valueize (valueize, gimple_assign_rhs2 (a1));
+ tree op21 = do_valueize (valueize, gimple_assign_rhs2 (a2));
if (!operand_equal_p (op11, op21))
return false;
if (invert_tree_comparison (gimple_assign_rhs_code (a1),
--
2.31.1
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH 2/2] Slightly improve bitwise_inverted_equal_p comparisons
2023-07-31 17:46 ` [PATCH 2/2] Slightly improve bitwise_inverted_equal_p comparisons Andrew Pinski
@ 2023-08-02 8:04 ` Richard Biener
0 siblings, 0 replies; 6+ messages in thread
From: Richard Biener @ 2023-08-02 8:04 UTC (permalink / raw)
To: Andrew Pinski; +Cc: gcc-patches
On Mon, Jul 31, 2023 at 7:47 PM Andrew Pinski via Gcc-patches
<gcc-patches@gcc.gnu.org> wrote:
>
> This slighly improves bitwise_inverted_equal_p
> for comparisons. Instead of just comparing the
> comparisons operands also valueize them.
> This will allow ccp and others to match the 2 comparisons
> without an extra pass happening.
>
> OK? Bootstrapped and tested on x86_64-linux-gnu.
OK.
> gcc/ChangeLog:
>
> * gimple-match-head.cc (gimple_bitwise_inverted_equal_p): Valueize
> the comparison operands before comparing them.
> ---
> gcc/gimple-match-head.cc | 8 ++++----
> 1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/gcc/gimple-match-head.cc b/gcc/gimple-match-head.cc
> index 0265e55be93..b1e96304d7c 100644
> --- a/gcc/gimple-match-head.cc
> +++ b/gcc/gimple-match-head.cc
> @@ -319,12 +319,12 @@ gimple_bitwise_inverted_equal_p (tree expr1, tree expr2, tree (*valueize) (tree)
> && TREE_CODE_CLASS (gimple_assign_rhs_code (a1)) == tcc_comparison
> && TREE_CODE_CLASS (gimple_assign_rhs_code (a2)) == tcc_comparison)
> {
> - tree op10 = gimple_assign_rhs1 (a1);
> - tree op20 = gimple_assign_rhs1 (a2);
> + tree op10 = do_valueize (valueize, gimple_assign_rhs1 (a1));
> + tree op20 = do_valueize (valueize, gimple_assign_rhs1 (a2));
> if (!operand_equal_p (op10, op20))
> return false;
> - tree op11 = gimple_assign_rhs2 (a1);
> - tree op21 = gimple_assign_rhs2 (a2);
> + tree op11 = do_valueize (valueize, gimple_assign_rhs2 (a1));
> + tree op21 = do_valueize (valueize, gimple_assign_rhs2 (a2));
> if (!operand_equal_p (op11, op21))
> return false;
> if (invert_tree_comparison (gimple_assign_rhs_code (a1),
> --
> 2.31.1
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 1/2] Move `~X & X` and `~X | X` over to use bitwise_inverted_equal_p
2023-07-31 17:46 [PATCH 1/2] Move `~X & X` and `~X | X` over to use bitwise_inverted_equal_p Andrew Pinski
2023-07-31 17:46 ` [PATCH 2/2] Slightly improve bitwise_inverted_equal_p comparisons Andrew Pinski
@ 2023-08-02 8:04 ` Richard Biener
2023-08-02 8:24 ` Jakub Jelinek
1 sibling, 1 reply; 6+ messages in thread
From: Richard Biener @ 2023-08-02 8:04 UTC (permalink / raw)
To: Andrew Pinski; +Cc: gcc-patches
On Mon, Jul 31, 2023 at 7:47 PM Andrew Pinski via Gcc-patches
<gcc-patches@gcc.gnu.org> wrote:
>
> This is a simple patch to move these 2 patterns over to use
> bitwise_inverted_equal_p. It also allows us to remove 2 other patterns
> which were used on comparisons as they are now handled by
> the original pattern.
>
> OK? Bootstrapped and tested on x86_64-linux-gnu with no regressions.
OK.
> gcc/ChangeLog:
>
> * match.pd (`~X & X`, `~X | X`): Move over to
> use bitwise_inverted_equal_p, removing :c as bitwise_inverted_equal_p
> handles that already.
> Remove range test simplifications to true/false as they
> are now handled by these patterns.
> ---
> gcc/match.pd | 28 ++++++----------------------
> 1 file changed, 6 insertions(+), 22 deletions(-)
>
> diff --git a/gcc/match.pd b/gcc/match.pd
> index 74f0a84f31d..7d030262698 100644
> --- a/gcc/match.pd
> +++ b/gcc/match.pd
> @@ -1157,8 +1157,9 @@ DEFINE_INT_AND_FLOAT_ROUND_FN (RINT)
>
> /* Simplify ~X & X as zero. */
> (simplify
> - (bit_and:c (convert? @0) (convert? (bit_not @0)))
> - { build_zero_cst (type); })
> + (bit_and (convert? @0) (convert? @1))
> + (if (bitwise_inverted_equal_p (@0, @1))
> + { build_zero_cst (type); }))
>
> /* PR71636: Transform x & ((1U << b) - 1) -> x & ~(~0U << b); */
> (simplify
> @@ -1395,8 +1396,9 @@ DEFINE_INT_AND_FLOAT_ROUND_FN (RINT)
> /* ~x ^ x -> -1 */
> (for op (bit_ior bit_xor)
> (simplify
> - (op:c (convert? @0) (convert? (bit_not @0)))
> - (convert { build_all_ones_cst (TREE_TYPE (@0)); })))
> + (op (convert? @0) (convert? @1))
> + (if (bitwise_inverted_equal_p (@0, @1))
> + (convert { build_all_ones_cst (TREE_TYPE (@0)); }))))
>
> /* x ^ x -> 0 */
> (simplify
> @@ -5994,24 +5996,6 @@ DEFINE_INT_AND_FLOAT_ROUND_FN (RINT)
> (bit_and:c (ordered @0 @0) (ordered:c@2 @0 @1))
> @2)
>
> -/* Simple range test simplifications. */
> -/* A < B || A >= B -> true. */
> -(for test1 (lt le le le ne ge)
> - test2 (ge gt ge ne eq ne)
> - (simplify
> - (bit_ior:c (test1 @0 @1) (test2 @0 @1))
> - (if (INTEGRAL_TYPE_P (TREE_TYPE (@0))
> - || VECTOR_INTEGER_TYPE_P (TREE_TYPE (@0)))
> - { constant_boolean_node (true, type); })))
> -/* A < B && A >= B -> false. */
> -(for test1 (lt lt lt le ne eq)
> - test2 (ge gt eq gt eq gt)
> - (simplify
> - (bit_and:c (test1 @0 @1) (test2 @0 @1))
> - (if (INTEGRAL_TYPE_P (TREE_TYPE (@0))
> - || VECTOR_INTEGER_TYPE_P (TREE_TYPE (@0)))
> - { constant_boolean_node (false, type); })))
> -
> /* A & (2**N - 1) <= 2**K - 1 -> A & (2**N - 2**K) == 0
> A & (2**N - 1) > 2**K - 1 -> A & (2**N - 2**K) != 0
>
> --
> 2.31.1
>
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH 1/2] Move `~X & X` and `~X | X` over to use bitwise_inverted_equal_p
2023-08-02 8:04 ` [PATCH 1/2] Move `~X & X` and `~X | X` over to use bitwise_inverted_equal_p Richard Biener
@ 2023-08-02 8:24 ` Jakub Jelinek
2023-08-02 22:11 ` Andrew Pinski
0 siblings, 1 reply; 6+ messages in thread
From: Jakub Jelinek @ 2023-08-02 8:24 UTC (permalink / raw)
To: Richard Biener; +Cc: Andrew Pinski, gcc-patches
On Wed, Aug 02, 2023 at 10:04:26AM +0200, Richard Biener via Gcc-patches wrote:
> > --- a/gcc/match.pd
> > +++ b/gcc/match.pd
> > @@ -1157,8 +1157,9 @@ DEFINE_INT_AND_FLOAT_ROUND_FN (RINT)
> >
> > /* Simplify ~X & X as zero. */
> > (simplify
> > - (bit_and:c (convert? @0) (convert? (bit_not @0)))
> > - { build_zero_cst (type); })
> > + (bit_and (convert? @0) (convert? @1))
> > + (if (bitwise_inverted_equal_p (@0, @1))
> > + { build_zero_cst (type); }))
I wonder if the above isn't incorrect.
Without the possibility of widening converts it would be ok,
but for widening conversions it is significant not just that
the bits of @0 and @1 are inverted, but also that they are either
both signed or both unsigned and so the MS bit (which is guaranteed
to be different) extends to 0s in one case and to all 1s in the other
one, so that even the upper bits are inverted.
But that isn't the case here. Something like (untested):
long long
foo (unsigned int x)
{
int y = x;
y = ~y;
return ((long long) x) & y;
}
Actually maybe for this pattern it happens to be ok, because while
the upper bits in this case might not be inverted between the extended
operands (if x has msb set), it will be 0 & 0 in the upper bits.
> >
> > /* PR71636: Transform x & ((1U << b) - 1) -> x & ~(~0U << b); */
> > (simplify
> > @@ -1395,8 +1396,9 @@ DEFINE_INT_AND_FLOAT_ROUND_FN (RINT)
> > /* ~x ^ x -> -1 */
> > (for op (bit_ior bit_xor)
> > (simplify
> > - (op:c (convert? @0) (convert? (bit_not @0)))
> > - (convert { build_all_ones_cst (TREE_TYPE (@0)); })))
> > + (op (convert? @0) (convert? @1))
> > + (if (bitwise_inverted_equal_p (@0, @1))
> > + (convert { build_all_ones_cst (TREE_TYPE (@0)); }))))
But not here.
long long
bar (unsigned int x)
{
int y = x;
y = ~y;
return ((long long) x) ^ y;
}
long long
baz (unsigned int x)
{
int y = x;
y = ~y;
return y ^ ((long long) x);
}
You pick TREE_TYPE (@0), but that is a random signedness if the two
operands have different signedness.
Jakub
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH 1/2] Move `~X & X` and `~X | X` over to use bitwise_inverted_equal_p
2023-08-02 8:24 ` Jakub Jelinek
@ 2023-08-02 22:11 ` Andrew Pinski
0 siblings, 0 replies; 6+ messages in thread
From: Andrew Pinski @ 2023-08-02 22:11 UTC (permalink / raw)
To: Jakub Jelinek; +Cc: Richard Biener, Andrew Pinski, gcc-patches
On Wed, Aug 2, 2023 at 1:25 AM Jakub Jelinek via Gcc-patches
<gcc-patches@gcc.gnu.org> wrote:
>
> On Wed, Aug 02, 2023 at 10:04:26AM +0200, Richard Biener via Gcc-patches wrote:
> > > --- a/gcc/match.pd
> > > +++ b/gcc/match.pd
> > > @@ -1157,8 +1157,9 @@ DEFINE_INT_AND_FLOAT_ROUND_FN (RINT)
> > >
> > > /* Simplify ~X & X as zero. */
> > > (simplify
> > > - (bit_and:c (convert? @0) (convert? (bit_not @0)))
> > > - { build_zero_cst (type); })
> > > + (bit_and (convert? @0) (convert? @1))
> > > + (if (bitwise_inverted_equal_p (@0, @1))
> > > + { build_zero_cst (type); }))
>
> I wonder if the above isn't incorrect.
> Without the possibility of widening converts it would be ok,
> but for widening conversions it is significant not just that
> the bits of @0 and @1 are inverted, but also that they are either
> both signed or both unsigned and so the MS bit (which is guaranteed
> to be different) extends to 0s in one case and to all 1s in the other
> one, so that even the upper bits are inverted.
> But that isn't the case here. Something like (untested):
> long long
> foo (unsigned int x)
> {
> int y = x;
> y = ~y;
> return ((long long) x) & y;
> }
> Actually maybe for this pattern it happens to be ok, because while
> the upper bits in this case might not be inverted between the extended
> operands (if x has msb set), it will be 0 & 0 in the upper bits.
>
> > >
> > > /* PR71636: Transform x & ((1U << b) - 1) -> x & ~(~0U << b); */
> > > (simplify
> > > @@ -1395,8 +1396,9 @@ DEFINE_INT_AND_FLOAT_ROUND_FN (RINT)
> > > /* ~x ^ x -> -1 */
> > > (for op (bit_ior bit_xor)
> > > (simplify
> > > - (op:c (convert? @0) (convert? (bit_not @0)))
> > > - (convert { build_all_ones_cst (TREE_TYPE (@0)); })))
> > > + (op (convert? @0) (convert? @1))
> > > + (if (bitwise_inverted_equal_p (@0, @1))
> > > + (convert { build_all_ones_cst (TREE_TYPE (@0)); }))))
>
> But not here.
> long long
> bar (unsigned int x)
> {
> int y = x;
> y = ~y;
> return ((long long) x) ^ y;
> }
>
> long long
> baz (unsigned int x)
> {
> int y = x;
> y = ~y;
> return y ^ ((long long) x);
> }
> You pick TREE_TYPE (@0), but that is a random signedness if the two
> operands have different signedness.
Oh you are correct, I am testing a patch which adds the test to make
sure the types of @0 and @1 match which brings us back to basically
was done beforehand and still provides the benefit of using
bitwise_inverted_equal_p for the comparisons.
Thanks,
Andrew
>
> Jakub
>
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2023-08-02 22:11 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-07-31 17:46 [PATCH 1/2] Move `~X & X` and `~X | X` over to use bitwise_inverted_equal_p Andrew Pinski
2023-07-31 17:46 ` [PATCH 2/2] Slightly improve bitwise_inverted_equal_p comparisons Andrew Pinski
2023-08-02 8:04 ` Richard Biener
2023-08-02 8:04 ` [PATCH 1/2] Move `~X & X` and `~X | X` over to use bitwise_inverted_equal_p Richard Biener
2023-08-02 8:24 ` Jakub Jelinek
2023-08-02 22:11 ` Andrew Pinski
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).