From: Richard Biener <rguenther@suse.de>
To: Tamar Christina <tamar.christina@arm.com>
Cc: gcc-patches@gcc.gnu.org, nd@arm.com, jeffreyalaw@gmail.com
Subject: Re: [PATCH]middle-end Recognize more conditional comparisons idioms.
Date: Mon, 26 Sep 2022 10:13:49 +0000 (UTC) [thread overview]
Message-ID: <nycvar.YFH.7.77.849.2209261006260.6652@jbgna.fhfr.qr> (raw)
In-Reply-To: <patch-16259-tamar@arm.com>
On Fri, 23 Sep 2022, Tamar Christina wrote:
> Hi All,
>
> GCC currently recognizes some of these for signed but not unsigned types.
> It also has trouble dealing with casts in between because these are handled
> by the fold machinery.
>
> This moves the pattern detection to match.pd instead.
where's the other copy and is it possible to remove it with this patch?
> We fold e.g.:
>
> uint32_t min1_32u(uint32_t a, uint32_t b, uint32_t c, uint32_t d) {
> uint32_t result;
> uint32_t m = (a >= b) - 1;
> result = (c & m) | (d & ~m);
> return result;
> }
>
> into a >= b ? c : d for all integral types.
>
> Bootstrapped Regtested on aarch64-none-linux-gnu and no issues.
>
> Ok for master?
>
> Thanks,
> Tamar
>
> gcc/ChangeLog:
>
> * match.pd: New cond select pattern.
>
> gcc/testsuite/ChangeLog:
>
> * gcc.dg/select_cond_1.c: New test.
> * gcc.dg/select_cond_2.c: New test.
> * gcc.dg/select_cond_3.c: New test.
>
> --- inline copy of patch --
> diff --git a/gcc/match.pd b/gcc/match.pd
> index 39da61bf117a6eb2924fc8a6473fb37ddadd60e9..7b8f50410acfd0afafc5606e972cfc4e125d3a5d 100644
> --- a/gcc/match.pd
> +++ b/gcc/match.pd
> @@ -3577,6 +3577,25 @@ DEFINE_INT_AND_FLOAT_ROUND_FN (RINT)
> (cond (le @0 integer_zerop@1) (negate@2 @0) integer_zerop@1)
> (max @2 @1))
>
> +/* (a & ((c `op` d) - 1)) | (b & ~((c `op` d) - 1)) -> c `op` d ? a : b. */
> +(for op (simple_comparison)
> + (simplify
> + (bit_xor:c
> + (convert2? @0)
> + (bit_and:c
> + (convert2? (bit_xor:c @1 @0))
> + (convert3? (negate (convert? (op@4 @2 @3))))))
I have a hard time matching this up with the comment above. It looks like
(T2)@0 ^ ((T2)(@1 ^ @0) & (T3)(-(T)(@2 <=> @3))
I suppose we've canonicalized (a & x) | (b & ~x) somehow.
It also looks like we move coversions inside a negate but not outside
or into a bit_and? Can we do a better job here to avoid the explosions
in the number of patterns?
> + /* Alternative form, where some canonicalization were not done due to the
> + arguments being signed. */
these two comment lines belong ...
> + (if (INTEGRAL_TYPE_P (type) && tree_zero_one_valued_p (@4))
> + (convert:type (cond @4 @1 @0))))
... here?
> + (simplify
> + (bit_ior:c
> + (mult:c @0 (convert (convert2? (op@4 @2 @3))))
> + (bit_and:c @1 (convert (plus:c integer_minus_onep (convert (op@4 @2 @3))))))
can you add a comment with what you match here as well? You are using
(op@4 @2 @3) twice, the point of the @4 capture is that the second
occurance could be just @4. I wonder how we arrived at the
multiplication here? That seems somewhat premature :/
Thanks,
Richard.
> + (if (INTEGRAL_TYPE_P (type) && tree_zero_one_valued_p (@4))
> + (cond @4 @0 @1))))
> +
> /* Simplifications of shift and rotates. */
>
> (for rotate (lrotate rrotate)
> diff --git a/gcc/testsuite/gcc.dg/select_cond_1.c b/gcc/testsuite/gcc.dg/select_cond_1.c
> new file mode 100644
> index 0000000000000000000000000000000000000000..9eb9959baafe5fffeec24e4e3ae656f8fcfe943c
> --- /dev/null
> +++ b/gcc/testsuite/gcc.dg/select_cond_1.c
> @@ -0,0 +1,97 @@
> +/* { dg-do run } */
> +/* { dg-additional-options "-O2 -std=c99 -fdump-tree-optimized -save-temps" } */
> +
> +#include <stdint.h>
> +
> +__attribute__((noipa, noinline))
> +uint32_t min1_32u(uint32_t a, uint32_t b, uint32_t c, uint32_t d) {
> + uint32_t result;
> + uint32_t m = (a >= b) - 1;
> + result = (c & m) | (d & ~m);
> + return result;
> +}
> +
> +__attribute__((noipa, noinline))
> +uint32_t max1_32u(uint32_t a, uint32_t b, uint32_t c, uint32_t d) {
> + uint32_t result;
> + uint32_t m = (a <= b) - 1;
> + result = (c & m) | (d & ~m);
> + return result;
> +}
> +
> +__attribute__((noipa, noinline))
> +uint32_t min2_32u(uint32_t a, uint32_t b, uint32_t c, uint32_t d) {
> + uint32_t result;
> + uint32_t m = (a > b) - 1;
> + result = (c & m) | (d & ~m);
> + return result;
> +}
> +
> +__attribute__((noipa, noinline))
> +uint32_t max2_32u(uint32_t a, uint32_t b, uint32_t c, uint32_t d) {
> + uint32_t result;
> + uint32_t m = (a < b) - 1;
> + result = (c & m) | (d & ~m);
> + return result;
> +}
> +
> +__attribute__((noipa, noinline))
> +uint32_t min3_32u(uint32_t a, uint32_t b, uint32_t c, uint32_t d) {
> + uint32_t result;
> + uint32_t m = (a == b) - 1;
> + result = (c & m) | (d & ~m);
> + return result;
> +}
> +
> +__attribute__((noipa, noinline))
> +uint32_t max3_32u(uint32_t a, uint32_t b, uint32_t c, uint32_t d) {
> + uint32_t result;
> + uint32_t m = (a != b) - 1;
> + result = (c & m) | (d & ~m);
> + return result;
> +}
> +
> +/* { dg-final { scan-tree-dump-times {_[0-9]+ \? c_[0-9]+\(D\) : d_[0-9]+\(D\)} 6 "optimized" } } */
> +
> +extern void abort ();
> +
> +int main () {
> +
> + if (min1_32u (3, 5, 7 , 8) != 7)
> + abort ();
> +
> + if (max1_32u (3, 5, 7 , 8) != 8)
> + abort ();
> +
> + if (min1_32u (5, 3, 7 , 8) != 8)
> + abort ();
> +
> + if (max1_32u (5, 3, 7 , 8) != 7)
> + abort ();
> +
> + if (min2_32u (3, 5, 7 , 8) != 7)
> + abort ();
> +
> + if (max2_32u (3, 5, 7 , 8) != 8)
> + abort ();
> +
> + if (min2_32u (5, 3, 7 , 8) != 8)
> + abort ();
> +
> + if (max2_32u (5, 3, 7 , 8) != 7)
> + abort ();
> +
> + if (min3_32u (3, 5, 7 , 8) != 7)
> + abort ();
> +
> + if (max3_32u (3, 5, 7 , 8) != 8)
> + abort ();
> +
> + if (min3_32u (5, 3, 7 , 8) != 7)
> + abort ();
> +
> + if (max3_32u (5, 3, 7 , 8) != 8)
> + abort ();
> +
> + return 0;
> +}
> diff --git a/gcc/testsuite/gcc.dg/select_cond_2.c b/gcc/testsuite/gcc.dg/select_cond_2.c
> new file mode 100644
> index 0000000000000000000000000000000000000000..623b2272ee7b4b00130d5e9fb8c781dbc5d4189e
> --- /dev/null
> +++ b/gcc/testsuite/gcc.dg/select_cond_2.c
> @@ -0,0 +1,99 @@
> +/* { dg-do run } */
> +/* { dg-additional-options "-O2 -std=c99 -fdump-tree-optimized -save-temps" } */
> +
> +#include <stdint.h>
> +
> +__attribute__((noipa, noinline))
> +uint32_t min1_32u(uint32_t a, uint32_t b) {
> + uint32_t result;
> + uint32_t m = (a >= b) - 1;
> + result = (a & m) | (b & ~m);
> + return result;
> +}
> +
> +__attribute__((noipa, noinline))
> +uint32_t max1_32u(uint32_t a, uint32_t b) {
> + uint32_t result;
> + uint32_t m = (a <= b) - 1;
> + result = (a & m) | (b & ~m);
> + return result;
> +}
> +
> +__attribute__((noipa, noinline))
> +uint32_t min2_32u(uint32_t a, uint32_t b) {
> + uint32_t result;
> + uint32_t m = (a > b) - 1;
> + result = (a & m) | (b & ~m);
> + return result;
> +}
> +
> +__attribute__((noipa, noinline))
> +uint32_t max2_32u(uint32_t a, uint32_t b) {
> + uint32_t result;
> + uint32_t m = (a < b) - 1;
> + result = (a & m) | (b & ~m);
> + return result;
> +}
> +
> +__attribute__((noipa, noinline))
> +uint32_t min3_32u(uint32_t a, uint32_t b) {
> + uint32_t result;
> + uint32_t m = (a == b) - 1;
> + result = (a & m) | (b & ~m);
> + return result;
> +}
> +
> +__attribute__((noipa, noinline))
> +uint32_t max3_32u(uint32_t a, uint32_t b) {
> + uint32_t result;
> + uint32_t m = (a != b) - 1;
> + result = (a & m) | (b & ~m);
> + return result;
> +}
> +
> +/* { dg-final { scan-tree-dump-not {_[0-9]+ \? c_[0-9]+\(D\) : d_[0-9]+\(D\)} "optimized" } } */
> +/* { dg-final { scan-tree-dump-times {MIN_EXPR} 2 "optimized" } } */
> +/* { dg-final { scan-tree-dump-times {MAX_EXPR} 2 "optimized" } } */
> +
> +extern void abort ();
> +
> +int main () {
> +
> + if (min1_32u (7 , 8) != 7)
> + abort ();
> +
> + if (max1_32u (7 , 8) != 8)
> + abort ();
> +
> + if (min1_32u (7 , 8) != 7)
> + abort ();
> +
> + if (max1_32u (7, 8) != 8)
> + abort ();
> +
> + if (min2_32u (7 , 8) != 7)
> + abort ();
> +
> + if (max2_32u (7 , 8) != 8)
> + abort ();
> +
> + if (min2_32u (7 , 8) != 7)
> + abort ();
> +
> + if (max2_32u (7 , 8) != 8)
> + abort ();
> +
> + if (min3_32u (7 , 8) != 7)
> + abort ();
> +
> + if (max3_32u (7 , 8) != 8)
> + abort ();
> +
> + if (min3_32u (7 , 8) != 7)
> + abort ();
> +
> + if (max3_32u (7 , 8) != 8)
> + abort ();
> +
> + return 0;
> +}
> diff --git a/gcc/testsuite/gcc.dg/select_cond_3.c b/gcc/testsuite/gcc.dg/select_cond_3.c
> new file mode 100644
> index 0000000000000000000000000000000000000000..754e73b121ce360564481b1fa7e1dd4fb27642a3
> --- /dev/null
> +++ b/gcc/testsuite/gcc.dg/select_cond_3.c
> @@ -0,0 +1,101 @@
> +/* { dg-do run } */
> +/* { dg-additional-options "-O2 -std=c99 -fdump-tree-optimized -save-temps" } */
> +
> +#include <stdint.h>
> +
> +__attribute__((noipa, noinline))
> +int32_t min1_32u1(int32_t a, int32_t b, int32_t c, int32_t d) {
> + uint32_t result;
> + uint32_t m = (a >= b) - 1;
> + result = (c & m) | (d & ~m);
> + return result;
> +}
> +
> +__attribute__((noipa, noinline))
> +uint32_t min1_32u2(uint32_t a, uint32_t b, int32_t c, int32_t d) {
> + uint32_t result;
> + uint32_t m = (a >= b) - 1;
> + result = (c & m) | (d & ~m);
> + return result;
> +}
> +
> +__attribute__((noipa, noinline))
> +uint32_t min1_32u3(uint32_t a, uint32_t b, uint32_t c, int32_t d) {
> + uint32_t result;
> + uint32_t m = (a >= b) - 1;
> + result = (c & m) | (d & ~m);
> + return result;
> +}
> +
> +__attribute__((noipa, noinline))
> +uint32_t min1_32u4(uint32_t a, uint32_t b, int32_t c, uint32_t d) {
> + uint32_t result;
> + uint32_t m = (a >= b) - 1;
> + result = (c & m) | (d & ~m);
> + return result;
> +}
> +
> +__attribute__((noipa, noinline))
> +uint32_t min1_32u5(uint32_t a, uint32_t b, uint32_t c, uint32_t d) {
> + uint32_t result;
> + uint32_t m = (a >= b) - 1;
> + result = (c & m) | (d & ~m);
> + return result;
> +}
> +
> +__attribute__((noipa, noinline))
> +uint32_t min1_32u6(uint32_t a, uint32_t b, uint32_t c, uint32_t d) {
> + uint32_t result;
> + int32_t m = (a >= b) - 1;
> + result = (c & m) | (d & ~m);
> + return result;
> +}
> +
> +__attribute__((noipa, noinline))
> +uint32_t min1_32u7(uint8_t a, uint16_t b, uint32_t c, uint32_t d) {
> + uint32_t result;
> + int32_t m = (a >= b) - 1;
> + result = (c & m) | (d & ~m);
> + return result;
> +}
> +
> +__attribute__((noipa, noinline))
> +uint32_t min1_32u8(uint32_t a, uint64_t b, uint32_t c, uint32_t d) {
> + uint32_t result;
> + int32_t m = (a >= b) - 1;
> + result = (c & m) | (d & ~m);
> + return result;
> +}
> +
> +/* { dg-final { scan-tree-dump-times {_[0-9]+ \? c[^ ]+ : d[^ ]+;} 8 "optimized" } } */
> +
> +extern void abort ();
> +
> +int main () {
> +
> + if (min1_32u1 (3, 5, 7 , 8) != 7)
> + abort ();
> +
> + if (min1_32u2 (3, 5, 7 , 8) != 7)
> + abort ();
> +
> + if (min1_32u3 (3, 5, 7 , 8) != 7)
> + abort ();
> +
> + if (min1_32u4 (3, 5, 7 , 8) != 7)
> + abort ();
> +
> + if (min1_32u5 (3, 5, 7 , 8) != 7)
> + abort ();
> +
> + if (min1_32u6 (3, 5, 7 , 8) != 7)
> + abort ();
> +
> + if (min1_32u7 (3, 5, 7 , 8) != 7)
> + abort ();
> +
> + if (min1_32u8 (3, 5, 7 , 8) != 7)
> + abort ();
> +
> + return 0;
> +}
>
>
>
>
>
--
Richard Biener <rguenther@suse.de>
SUSE Software Solutions Germany GmbH, Frankenstrasse 146, 90461 Nuernberg,
Germany; GF: Ivo Totev, Andrew Myers, Andrew McDonald, Boudien Moerman;
HRB 36809 (AG Nuernberg)
next prev parent reply other threads:[~2022-09-26 10:13 UTC|newest]
Thread overview: 4+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-09-23 9:18 Tamar Christina
2022-09-26 10:13 ` Richard Biener [this message]
2022-10-31 11:47 ` Tamar Christina
2022-11-07 13:17 ` Richard Biener
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=nycvar.YFH.7.77.849.2209261006260.6652@jbgna.fhfr.qr \
--to=rguenther@suse.de \
--cc=gcc-patches@gcc.gnu.org \
--cc=jeffreyalaw@gmail.com \
--cc=nd@arm.com \
--cc=tamar.christina@arm.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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).