From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from smtp-out2.suse.de (smtp-out2.suse.de [IPv6:2001:67c:2178:6::1d]) by sourceware.org (Postfix) with ESMTPS id DDCC83858D32 for ; Mon, 26 Sep 2022 10:42:34 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org DDCC83858D32 Authentication-Results: sourceware.org; dmarc=pass (p=none dis=none) header.from=suse.de Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=suse.de Received: from relay2.suse.de (relay2.suse.de [149.44.160.134]) by smtp-out2.suse.de (Postfix) with ESMTP id BAEB41FA4F; Mon, 26 Sep 2022 10:42:33 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=suse.de; s=susede2_rsa; t=1664188953; h=from:from:reply-to:date:date:message-id:message-id:to:to:cc:cc: mime-version:mime-version:content-type:content-type: in-reply-to:in-reply-to:references:references; bh=vza67NX/AiL8W1mOcXEZospk+rZf+aRu5KdXBkpvpmg=; b=eX/hKOL5G1ssdnvo1EkBS2oSyx2RWOyLNOJIr4DVWRZGFGenymx/raAEEW5CnYWxyfQNzw X2GUMLURIN514BjYXPK76pBFQhPTb2La0QGq23dLeIASsMF9xJ/QSjxFS72YqNTQPhdXtQ P1rHqRi8hVrP44FmjvMlYiMng7l+h2U= DKIM-Signature: v=1; a=ed25519-sha256; c=relaxed/relaxed; d=suse.de; s=susede2_ed25519; t=1664188953; h=from:from:reply-to:date:date:message-id:message-id:to:to:cc:cc: mime-version:mime-version:content-type:content-type: in-reply-to:in-reply-to:references:references; bh=vza67NX/AiL8W1mOcXEZospk+rZf+aRu5KdXBkpvpmg=; b=f1v3vGlSabF7oB9y9MCbk2UZkYz12/3E4f4qJmv5H0vviamJtKibRIrTGP0qODx/OZRtgq MahKQwYku9c37CBA== Received: from wotan.suse.de (wotan.suse.de [10.160.0.1]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by relay2.suse.de (Postfix) with ESMTPS id B181B2C14F; Mon, 26 Sep 2022 10:42:33 +0000 (UTC) Date: Mon, 26 Sep 2022 10:42:33 +0000 (UTC) From: Richard Biener To: Tamar Christina cc: Andrew Pinski , nd , "gcc-patches@gcc.gnu.org" Subject: RE: [PATCH]middle-end simplify complex if expressions where comparisons are inverse of one another. In-Reply-To: Message-ID: References: User-Agent: Alpine 2.22 (LSU 394 2020-01-19) MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII X-Spam-Status: No, score=-11.1 required=5.0 tests=BAYES_00,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,GIT_PATCH_0,KAM_SHORT,RCVD_IN_DNSWL_NONE,SPF_HELO_NONE,SPF_PASS,TXREP autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org List-Id: On Fri, 23 Sep 2022, Tamar Christina wrote: > > -----Original Message----- > > From: Richard Biener > > Sent: Friday, September 23, 2022 9:10 AM > > To: Tamar Christina > > Cc: Andrew Pinski ; nd ; gcc- > > patches@gcc.gnu.org > > Subject: RE: [PATCH]middle-end simplify complex if expressions where > > comparisons are inverse of one another. > > > > On Fri, 23 Sep 2022, Tamar Christina wrote: > > > > > Hello, > > > > > > > where logical_inverted is somewhat contradicting using > > > > zero_one_valued instead of truth_valued_p (I think the former might > > > > not work for vector booleans?). > > > > > > > > In the end I'd prefer zero_one_valued_p but avoiding > > > > inverse_conditions_p would be nice. > > > > > > > > Richard. > > > > > > It's not pretty but I've made it work and added more tests. > > > > > > Bootstrapped Regtested on aarch64-none-linux-gnu, x86_64-pc-linux-gnu > > > and no issues. > > > > > > Ok for master? > > > > > > Thanks, > > > Tamar > > > > > > gcc/ChangeLog: > > > > > > * match.pd: Add new rule. > > > > > > gcc/testsuite/ChangeLog: > > > > > > * gcc.target/aarch64/if-compare_1.c: New test. > > > * gcc.target/aarch64/if-compare_2.c: New test. > > > > > > --- inline copy of patch --- > > > > > > diff --git a/gcc/match.pd b/gcc/match.pd index > > > > > b61ed70e69b881a49177f10f20c1f92712bb8665..39da61bf117a6eb2924fc8a647 > > 3f > > > b37ddadd60e9 100644 > > > --- a/gcc/match.pd > > > +++ b/gcc/match.pd > > > @@ -1903,6 +1903,101 @@ DEFINE_INT_AND_FLOAT_ROUND_FN (RINT) > > > (if (INTEGRAL_TYPE_P (type)) > > > (bit_and @0 @1))) > > > > > > +(for cmp (tcc_comparison) > > > + icmp (inverted_tcc_comparison) > > > + /* Fold (((a < b) & c) | ((a >= b) & d)) into (a < b ? c : d) & 1. > > > +*/ (simplify > > > + (bit_ior > > > + (bit_and:c (convert? zero_one_valued_p@0) @2) > > > + (bit_and:c (convert? zero_one_valued_p@1) @3)) > > > + (with { > > > + enum tree_code c1 > > > + = (TREE_CODE (@0) == SSA_NAME > > > + ? gimple_assign_rhs_code (SSA_NAME_DEF_STMT (@0)) : > > TREE_CODE > > > +(@0)); > > > + > > > + enum tree_code c2 > > > + = (TREE_CODE (@1) == SSA_NAME > > > + ? gimple_assign_rhs_code (SSA_NAME_DEF_STMT (@1)) : > > TREE_CODE (@1)); > > > + } > > > + (if (INTEGRAL_TYPE_P (type) > > > + && c1 == cmp > > > + && c2 == icmp > > > > So that doesn't have any advantage over doing > > > > (simplify > > (bit_ior > > (bit_and:c (convert? (cmp@0 @01 @02)) @2) > > (bit_and:c (convert? (icmp@1 @11 @12)) @3)) ... > > > > I don't remember if that's what we had before. > > No, the specific problem has always been applying zero_one_valued_p to the right type. > Before it was much shorter because I was using the tree helper function to get the inverses. > > But with your suggestion I think I can do zero_one_valued_p on @0 and @1 instead.. But with comparsions and INTEGRAL_TYPE_P the value is always zero or one so I'm confused. > > > > > + /* The scalar version has to be canonicalized after vectorization > > > + because it makes unconditional loads conditional ones, which > > > + means we lose vectorization because the loads may trap. */ > > > + && canonicalize_math_after_vectorization_p ()) > > > + (bit_and (cond @0 @2 @3) { build_one_cst (type); })))) > > > + > > > + /* Fold ((-(a < b) & c) | (-(a >= b) & d)) into a < b ? c : d. */ > > > > The comment doesn't match the pattern below? > > The pattern in the comment gets rewritten to this form eventually, > so I match it instead. I can update the comment but I thought the above > made it more clear why these belong together ? Please mention the canonicalized form in the comment as well. > > > > > + (simplify > > > + (bit_ior > > > + (cond zero_one_valued_p@0 @2 zerop) > > > + (cond zero_one_valued_p@1 @3 zerop)) > > > + (with { > > > + enum tree_code c1 > > > + = (TREE_CODE (@0) == SSA_NAME > > > + ? gimple_assign_rhs_code (SSA_NAME_DEF_STMT (@0)) : > > TREE_CODE > > > +(@0)); > > > + > > > + enum tree_code c2 > > > + = (TREE_CODE (@1) == SSA_NAME > > > + ? gimple_assign_rhs_code (SSA_NAME_DEF_STMT (@1)) : > > TREE_CODE (@1)); > > > + } > > > + (if (INTEGRAL_TYPE_P (type) > > > + && c1 == cmp > > > + && c2 == icmp > > > + /* The scalar version has to be canonicalized after vectorization > > > + because it makes unconditional loads conditional ones, which > > > + means we lose vectorization because the loads may trap. */ > > > + && canonicalize_math_after_vectorization_p ()) > > > + (cond @0 @2 @3)))) > > > + > > > + /* Vector Fold (((a < b) & c) | ((a >= b) & d)) into a < b ? c : d. > > > + and ((~(a < b) & c) | (~(a >= b) & d)) into a < b ? c : d. */ > > > +(simplify > > > + (bit_ior > > > + (bit_and:c (vec_cond:s @0 @4 @5) @2) > > > + (bit_and:c (vec_cond:s @1 @4 @5) @3)) > > > + (with { > > > + enum tree_code c1 > > > + = (TREE_CODE (@0) == SSA_NAME > > > + ? gimple_assign_rhs_code (SSA_NAME_DEF_STMT (@0)) : > > TREE_CODE > > > +(@0)); > > > + > > > + enum tree_code c2 > > > + = (TREE_CODE (@1) == SSA_NAME > > > + ? gimple_assign_rhs_code (SSA_NAME_DEF_STMT (@1)) : > > TREE_CODE (@1)); > > > + } > > > + (if (c1 == cmp && c2 == icmp) > > > + (if (integer_zerop (@5)) > > > + (switch > > > + (if (integer_onep (@4)) > > > + (bit_and (vec_cond @0 @2 @3) @4)) > > > + (if (integer_minus_onep (@4)) > > > + (vec_cond @0 @2 @3))) > > > + (if (integer_zerop (@4)) > > > + (switch > > > + (if (integer_onep (@5)) > > > + (bit_and (vec_cond @0 @3 @2) @5)) > > > + (if (integer_minus_onep (@5)) > > > + (vec_cond @0 @3 @2)))))))) > > > + > > > + /* Scalar Vectorized Fold ((-(a < b) & c) | (-(a >= b) & d)) > > > + into a < b ? d : c. */ > > > + (simplify > > > + (bit_ior > > > + (vec_cond:s @0 @2 integer_zerop) > > > + (vec_cond:s @1 @3 integer_zerop)) > > > + (with { > > > + enum tree_code c1 > > > + = (TREE_CODE (@0) == SSA_NAME > > > + ? gimple_assign_rhs_code (SSA_NAME_DEF_STMT (@0)) : > > TREE_CODE > > > +(@0)); > > > + > > > + enum tree_code c2 > > > + = (TREE_CODE (@1) == SSA_NAME > > > + ? gimple_assign_rhs_code (SSA_NAME_DEF_STMT (@1)) : > > TREE_CODE (@1)); > > > + } > > > + (if (c1 == cmp && c2 == icmp) > > > + (vec_cond @0 @2 @3))))) > > > + > > > > As you say, it's not pretty. When looking at > > > > int zoo1 (int a, int b, int c, int d) > > { > > return ((a < b) & c) | ((a >= b) & d); } > > > > I see that we fail to transform this to > > > > _Bool tem = a < b; > > return (tem & c) | (!tem & d); > > > > but when feeding that in as source then forwprop undoes this transform. > > Still when in this form the patterns would be a lot easier to handle? > > Hmm perhaps, that would remove the need for icmp, but with your suggestion above I think > I can clean this up, let me give it a try. Sure. Thanks, Richard. > Tamar. > > > > > Richard. > > > > > > > /* Transform X & -Y into X * Y when Y is { 0 or 1 }. */ (simplify > > > (bit_and:c (convert? (negate zero_one_valued_p@0)) @1) diff --git > > > a/gcc/testsuite/gcc.target/aarch64/if-compare_1.c > > > b/gcc/testsuite/gcc.target/aarch64/if-compare_1.c > > > new file mode 100644 > > > index > > > > > 0000000000000000000000000000000000000000..53bbd779a30e1a30e0ce0e4e5 > > eaf > > > 589bfaf570fe > > > --- /dev/null > > > +++ b/gcc/testsuite/gcc.target/aarch64/if-compare_1.c > > > @@ -0,0 +1,47 @@ > > > +/* { dg-do run } */ > > > +/* { dg-additional-options "-O -save-temps" } */ > > > +/* { dg-final { check-function-bodies "**" "" "" { target { le } } } > > > +} */ > > > + > > > +extern void abort (); > > > + > > > +/* > > > +**zoo1: > > > +** cmp w0, w1 > > > +** csel w0, w2, w3, lt > > > +** and w0, w0, 1 > > > +** ret > > > +*/ > > > +__attribute((noipa, noinline)) > > > +int zoo1 (int a, int b, int c, int d) { > > > + return ((a < b) & c) | ((a >= b) & d); } > > > + > > > +/* > > > +**zoo2: > > > +** cmp w0, w1 > > > +** csel w0, w2, w3, lt > > > +** ret > > > +*/ > > > +__attribute((noipa, noinline)) > > > +int zoo2 (int a, int b, int c, int d) > > > +{ > > > + return (-(a < b) & c) | (-(a >= b) & d); > > > +} > > > + > > > +int main () > > > +{ > > > + if (zoo1 (-3, 3, 5, 8) != 1) > > > + abort (); > > > + > > > + if (zoo1 (3, -3, 5, 8) != 0) > > > + abort (); > > > + > > > + if (zoo2 (-3, 3, 5, 8) != 5) > > > + abort (); > > > + > > > + if (zoo2 (3, -3, 5, 8) != 8) > > > + abort (); > > > + > > > + return 0; > > > +} > > > diff --git a/gcc/testsuite/gcc.target/aarch64/if-compare_2.c > > b/gcc/testsuite/gcc.target/aarch64/if-compare_2.c > > > new file mode 100644 > > > index > > 0000000000000000000000000000000000000000..14988abac45989578b198f28c7 > > c0ea203959c08b > > > --- /dev/null > > > +++ b/gcc/testsuite/gcc.target/aarch64/if-compare_2.c > > > @@ -0,0 +1,96 @@ > > > +/* { dg-do run } */ > > > +/* { dg-additional-options "-O3 -std=c99 -save-temps" } */ > > > +/* { dg-final { check-function-bodies "**" "" "" { target { le } } } } */ > > > + > > > +#pragma GCC target "+nosve" > > > + > > > +#include > > > + > > > +typedef int v4si __attribute__ ((vector_size (16))); > > > + > > > +/* > > > +**foo1: > > > +** cmgt v0.4s, v1.4s, v0.4s > > > +** bsl v0.16b, v2.16b, v3.16b > > > +** ret > > > +*/ > > > +v4si foo1 (v4si a, v4si b, v4si c, v4si d) { > > > + return ((a < b) & c) | ((a >= b) & d); > > > +} > > > + > > > +/* > > > +**foo2: > > > +** cmgt v0.4s, v1.4s, v0.4s > > > +** bsl v0.16b, v3.16b, v2.16b > > > +** ret > > > +*/ > > > +v4si foo2 (v4si a, v4si b, v4si c, v4si d) { > > > + return (~(a < b) & c) | (~(a >= b) & d); > > > +} > > > + > > > + > > > +/** > > > +**bar1: > > > +**... > > > +** cmge v[0-9]+.4s, v[0-9]+.4s, v[0-9]+.4s > > > +** bsl v[0-9]+.16b, v[0-9]+.16b, v[0-9]+.16b > > > +** and v[0-9]+.16b, v[0-9]+.16b, v[0-9]+.16b > > > +**... > > > +*/ > > > +void bar1 (int * restrict a, int * restrict b, int * restrict c, > > > + int * restrict d, int * restrict res, int n) > > > +{ > > > + for (int i = 0; i < (n & -4); i++) > > > + res[i] = ((a[i] < b[i]) & c[i]) | ((a[i] >= b[i]) & d[i]); > > > +} > > > + > > > +/** > > > +**bar2: > > > +**... > > > +** cmge v[0-9]+.4s, v[0-9]+.4s, v[0-9]+.4s > > > +** bsl v[0-9]+.16b, v[0-9]+.16b, v[0-9]+.16b > > > +**... > > > +*/ > > > +void bar2 (int * restrict a, int * restrict b, int * restrict c, > > > + int * restrict d, int * restrict res, int n) > > > +{ > > > + for (int i = 0; i < (n & -4); i++) > > > + res[i] = (-(a[i] < b[i]) & c[i]) | (-(a[i] >= b[i]) & d[i]); > > > +} > > > + > > > +extern void abort (); > > > + > > > +int main () > > > +{ > > > + > > > + v4si a = { -3, -3, -3, -3 }; > > > + v4si b = { 3, 3, 3, 3 }; > > > + v4si c = { 5, 5, 5, 5 }; > > > + v4si d = { 8, 8, 8, 8 }; > > > + > > > + v4si res1 = foo1 (a, b, c, d); > > > + if (memcmp (&res1, &c, 16UL) != 0) > > > + abort (); > > > + > > > + v4si res2 = foo2 (a, b, c, d); > > > + if (memcmp (&res2, &d, 16UL) != 0) > > > + abort (); > > > + > > > + int ar[4] = { -3, -3, -3, -3 }; > > > + int br[4] = { 3, 3, 3, 3 }; > > > + int cr[4] = { 5, 5, 5, 5 }; > > > + int dr[4] = { 8, 8, 8, 8 }; > > > + > > > + int exp1[4] = { 1, 1, 1, 1 }; > > > + int res3[4]; > > > + bar1 ((int*)&ar, (int*)&br, (int*)&cr, (int*)&dr, (int*)&res3, 4); > > > + if (memcmp (&res3, &exp1, 16UL) != 0) > > > + abort (); > > > + > > > + int res4[4]; > > > + bar2 ((int*)&ar, (int*)&br, (int*)&cr, (int*)&dr, (int*)&res4, 4); > > > + if (memcmp (&res4, &cr, 16UL) != 0) > > > + abort (); > > > + > > > + return 0; > > > +} > > > > > > > > > > > > > -- > > Richard Biener > > SUSE Software Solutions Germany GmbH, Frankenstrasse 146, 90461 > > Nuernberg, > > Germany; GF: Ivo Totev, Andrew Myers, Andrew McDonald, Boudien > > Moerman; > > HRB 36809 (AG Nuernberg) > -- Richard Biener SUSE Software Solutions Germany GmbH, Frankenstrasse 146, 90461 Nuernberg, Germany; GF: Ivo Totev, Andrew Myers, Andrew McDonald, Boudien Moerman; HRB 36809 (AG Nuernberg)