From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mx0a-001b2d01.pphosted.com (mx0b-001b2d01.pphosted.com [148.163.158.5]) by sourceware.org (Postfix) with ESMTPS id 3A2A83840C36 for ; Thu, 27 Aug 2020 20:47:32 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.3.2 sourceware.org 3A2A83840C36 Received: from pps.filterd (m0098414.ppops.net [127.0.0.1]) by mx0b-001b2d01.pphosted.com (8.16.0.42/8.16.0.42) with SMTP id 07RKWNxM039011; Thu, 27 Aug 2020 16:47:32 -0400 Received: from pps.reinject (localhost [127.0.0.1]) by mx0b-001b2d01.pphosted.com with ESMTP id 336k219uy3-1 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=NOT); Thu, 27 Aug 2020 16:47:31 -0400 Received: from m0098414.ppops.net (m0098414.ppops.net [127.0.0.1]) by pps.reinject (8.16.0.36/8.16.0.36) with SMTP id 07RKfvsG076080; Thu, 27 Aug 2020 16:47:31 -0400 Received: from ppma05wdc.us.ibm.com (1b.90.2fa9.ip4.static.sl-reverse.com [169.47.144.27]) by mx0b-001b2d01.pphosted.com with ESMTP id 336k219uxm-1 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=NOT); Thu, 27 Aug 2020 16:47:31 -0400 Received: from pps.filterd (ppma05wdc.us.ibm.com [127.0.0.1]) by ppma05wdc.us.ibm.com (8.16.0.42/8.16.0.42) with SMTP id 07RKkuGu009843; Thu, 27 Aug 2020 20:47:30 GMT Received: from b03cxnp08028.gho.boulder.ibm.com (b03cxnp08028.gho.boulder.ibm.com [9.17.130.20]) by ppma05wdc.us.ibm.com with ESMTP id 332uw7vs2y-1 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=NOT); Thu, 27 Aug 2020 20:47:30 +0000 Received: from b03ledav002.gho.boulder.ibm.com (b03ledav002.gho.boulder.ibm.com [9.17.130.233]) by b03cxnp08028.gho.boulder.ibm.com (8.14.9/8.14.9/NCO v10.0) with ESMTP id 07RKlT4v55312714 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-GCM-SHA384 bits=256 verify=OK); Thu, 27 Aug 2020 20:47:29 GMT Received: from b03ledav002.gho.boulder.ibm.com (unknown [127.0.0.1]) by IMSVA (Postfix) with ESMTP id 723C3136051; Thu, 27 Aug 2020 20:47:29 +0000 (GMT) Received: from b03ledav002.gho.boulder.ibm.com (unknown [127.0.0.1]) by IMSVA (Postfix) with ESMTP id 724DF13604F; Thu, 27 Aug 2020 20:47:28 +0000 (GMT) Received: from sig-9-65-193-110.ibm.com (unknown [9.65.193.110]) by b03ledav002.gho.boulder.ibm.com (Postfix) with ESMTP; Thu, 27 Aug 2020 20:47:28 +0000 (GMT) Message-ID: <02775eb3d249160a906a9813b444ce3b8a7265f3.camel@vnet.ibm.com> Subject: Re: [PATCH 4/4] PowerPC: Add power10 xscmp{eq,gt,ge}qp support From: will schmidt To: Michael Meissner , gcc-patches@gcc.gnu.org, Segher Boessenkool , David Edelsohn , Bill Schmidt , Peter Bergner Date: Thu, 27 Aug 2020 15:47:27 -0500 In-Reply-To: <20200827024637.GD21803@ibm-toto.the-meissners.org> References: <20200827024142.GA15560@ibm-toto.the-meissners.org> <20200827024637.GD21803@ibm-toto.the-meissners.org> Content-Type: text/plain; charset="UTF-8" X-Mailer: Evolution 3.28.5 (3.28.5-8.el7) Mime-Version: 1.0 Content-Transfer-Encoding: 7bit X-TM-AS-GCONF: 00 X-Proofpoint-Virus-Version: vendor=fsecure engine=2.50.10434:6.0.235, 18.0.687 definitions=2020-08-27_12:2020-08-27, 2020-08-27 signatures=0 X-Proofpoint-Spam-Details: rule=outbound_notspam policy=outbound score=0 bulkscore=0 lowpriorityscore=0 adultscore=0 clxscore=1015 impostorscore=0 priorityscore=1501 spamscore=0 mlxscore=0 phishscore=0 suspectscore=4 mlxlogscore=999 malwarescore=0 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.12.0-2006250000 definitions=main-2008270151 X-Spam-Status: No, score=-11.6 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_EF, GIT_PATCH_0, RCVD_IN_DNSWL_LOW, RCVD_IN_MSPIKE_H2, SPF_HELO_NONE, SPF_PASS, TXREP autolearn=ham autolearn_force=no version=3.4.2 X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) on server2.sourceware.org X-BeenThere: gcc-patches@gcc.gnu.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Gcc-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 27 Aug 2020 20:47:34 -0000 On Wed, 2020-08-26 at 22:46 -0400, Michael Meissner via Gcc-patches wrote: > PowerPC: Add power10 xscmp{eq,gt,ge}qp support. > > This patch adds the conditional move support. In adding the conditional move > support, the optimizers will be able to convert things like: > > a = (b > c) ? b : c; > > into the instructions. This patch merges together the scalar SF/DF conditional > move with the scalar KF/TF conditional move. It extends the optimization that > was previously used for SFmode and DFmode to allow the comparison to be a > different scalar floating point mode than the move. I.e. > > __float128 a, b, c; > float x, y; > > /* ... */ > > a = (x == y) ? b : c; > > I did have to add an XXPERMDI if the comparison mode was SFmode or DFmode, and > the move mode is KFmode or TFmode (the XSCMP{EQ,GT,GE}DP instructions > explicitly set the bottom 64 bits of the vector register to 0). > > I have built compilers on a little endian power9 Linux system with all 4 > patches applied. I did bootstrap builds and ran the testsuite, with no > regressions. Previous versions of the patch was also tested on a little endian > power8 Linux system. I would like to check this patch into the master branch > for GCC 11. At this time, I do not anticipate needing to backport these > changes to GCC 10.3. > > gcc/ > 2020-08-26 Michael Meissner > > * config/rs6000/predicates.md (fpmask_normal_or_invert_operator): > New predicate. > * config/rs6000/rs6000.c (have_compare_and_set_mask): Add IEEE > 128-bit floating point types. > * config/rs6000/rs6000.md (FSCALAR2): New iterator for floating > point conditional moves. > (movcc_p9): Replace with > mov. (movcc): Update to use FSCALAR iterator > (movcc_invert_p9): Replace with > mov. > (mov): Combine both If i followed correctly, that should that be *movcc " ..? > movcc_p9 and > movcc_invert_p9 patterns. Add ISA 3.1 > support for IEEE 128-bit conditional moves. Always use an > earlyclobber register for the mask. Use XXPERMDI to extend the > mask if we have a 64-bit comparison and 128-bit move. > register for the mask. ok > (fpmask, xxsel): Add ISA 3.1 support for IEEE 128-bit > conditional moves. Enable the generator functionality so > mov can call it. Update constraints > for 128-bit operations. ok reviewed the rest, nothing else jumped out at me. lgtm, thanks -Will > > gcc/testsuite/ > 2020-08-26 Michael Meissner > > * gcc.target/powerpc/float128-cmove.c: New test. > * gcc.target/powerpc/float128-minmax-3.c: New test. > > --- > gcc/config/rs6000/predicates.md | 5 + > gcc/config/rs6000/rs6000.c | 4 + > gcc/config/rs6000/rs6000.md | 154 ++++++++++-------- > .../gcc.target/powerpc/float128-cmove.c | 93 +++++++++++ > .../gcc.target/powerpc/float128-minmax-3.c | 15 ++ > 5 files changed, 200 insertions(+), 71 deletions(-) > create mode 100644 gcc/testsuite/gcc.target/powerpc/float128-cmove.c > create mode 100644 gcc/testsuite/gcc.target/powerpc/float128-minmax-3.c > > diff --git a/gcc/config/rs6000/predicates.md b/gcc/config/rs6000/predicates.md > index 2709e46f7e5..60b45601e9b 100644 > --- a/gcc/config/rs6000/predicates.md > +++ b/gcc/config/rs6000/predicates.md > @@ -1225,6 +1225,11 @@ (define_predicate "fpmask_comparison_operator" > (define_predicate "invert_fpmask_comparison_operator" > (match_code "ne,unlt,unle")) > > +;; Return 1 if OP is either a fpmask_comparison_operator or > +;; invert_fpmask_comparsion_operator. > +(define_predicate "fpmask_normal_or_invert_operator" > + (match_code "eq,gt,ge,ne,unlt,unle")) > + > ;; Return 1 if OP is a comparison operation suitable for integer vector/scalar > ;; comparisons that generate a -1/0 mask. > (define_predicate "vecint_comparison_operator" > diff --git a/gcc/config/rs6000/rs6000.c b/gcc/config/rs6000/rs6000.c > index 05eb141a2cd..403897926c5 100644 > --- a/gcc/config/rs6000/rs6000.c > +++ b/gcc/config/rs6000/rs6000.c > @@ -15177,6 +15177,10 @@ have_compare_and_set_mask (machine_mode mode) > case DFmode: > return TARGET_P9_MINMAX; > > + case KFmode: > + case TFmode: > + return FLOAT128_IEEE_MINMAX_P (mode); > + > default: > break; > } > diff --git a/gcc/config/rs6000/rs6000.md b/gcc/config/rs6000/rs6000.md > index 006e60f09bc..147c635994c 100644 > --- a/gcc/config/rs6000/rs6000.md > +++ b/gcc/config/rs6000/rs6000.md > @@ -795,6 +795,15 @@ (define_mode_iterator FSCALAR [SF > (KF "FLOAT128_IEEE_MINMAX_P (KFmode)") > (TF "FLOAT128_IEEE_MINMAX_P (TFmode)")]) > > +;; Secondary iterator for scalar binary floating point operations. This is > +;; used for the conditional moves when we have a compare and set mask > +;; instruction. Using this instruction allows us to do a conditional move > +;; where the comparison type might be different from the values being moved. > +(define_mode_iterator FSCALAR2 [SF > + DF > + (KF "FLOAT128_IEEE_MINMAX_P (KFmode)") > + (TF "FLOAT128_IEEE_MINMAX_P (TFmode)")]) > + > ;; Constraints to use for scalar FP operations > (define_mode_attr Fm [(SF "wa") > (DF "wa") > @@ -5290,10 +5299,10 @@ (define_insn "*setnbcr_signed_" > > ;; Floating point conditional move > (define_expand "movcc" > - [(set (match_operand:SFDF 0 "gpc_reg_operand") > - (if_then_else:SFDF (match_operand 1 "comparison_operator") > - (match_operand:SFDF 2 "gpc_reg_operand") > - (match_operand:SFDF 3 "gpc_reg_operand")))] > + [(set (match_operand:FSCALAR 0 "gpc_reg_operand") > + (if_then_else:FSCALAR (match_operand 1 "comparison_operator") > + (match_operand:FSCALAR 2 "gpc_reg_operand") > + (match_operand:FSCALAR 3 "gpc_reg_operand")))] > "TARGET_HARD_FLOAT && TARGET_PPC_GFXOPT" > { > if (rs6000_emit_cmove (operands[0], operands[1], operands[2], operands[3])) > @@ -5313,92 +5322,95 @@ (define_insn "*fsel4" > "fsel %0,%1,%2,%3" > [(set_attr "type" "fp")]) > > -(define_insn_and_split "*movcc_p9" > - [(set (match_operand:SFDF 0 "vsx_register_operand" "=&,") > - (if_then_else:SFDF > - (match_operator:CCFP 1 "fpmask_comparison_operator" > - [(match_operand:SFDF2 2 "vsx_register_operand" ",") > - (match_operand:SFDF2 3 "vsx_register_operand" ",")]) > - (match_operand:SFDF 4 "vsx_register_operand" ",") > - (match_operand:SFDF 5 "vsx_register_operand" ","))) > - (clobber (match_scratch:V2DI 6 "=0,&wa"))] > +;; Conditional moves using scalar floating point types if we have a compare and > +;; set mask instruction (like xscmpeqdp). > +;; > +;; If we have a 64-bit comparison and a 128-bit mode, such as: > +;; > +;; double x, y; > +;; __float128 a, b, c; > +;; a = (x == y) ? b : c; > +;; > +;; We need to extend the comparison (XSCMPEQDP puts 0's in the bottom 64-bits). > +(define_insn_and_split "*movcc" > + [(set (match_operand:FSCALAR 0 "vsx_register_operand" "=") > + (if_then_else:FSCALAR > + (match_operator:CCFP 1 "fpmask_normal_or_invert_operator" > + [(match_operand:FSCALAR2 2 "vsx_register_operand" "") > + (match_operand:FSCALAR2 3 "vsx_register_operand" "")]) > + (match_operand:FSCALAR 4 "vsx_register_operand" "") > + (match_operand:FSCALAR 5 "vsx_register_operand" ""))) > + (clobber (match_scratch:V2DI 6 "="))] > "TARGET_P9_MINMAX" > "#" > - "" > - [(set (match_dup 6) > - (if_then_else:V2DI (match_dup 1) > - (match_dup 7) > - (match_dup 8))) > - (set (match_dup 0) > - (if_then_else:SFDF (ne (match_dup 6) > - (match_dup 8)) > - (match_dup 4) > - (match_dup 5)))] > + "&& 1" > + [(pc)] > { > - if (GET_CODE (operands[6]) == SCRATCH) > - operands[6] = gen_reg_rtx (V2DImode); > + rtx dest = operands[0]; > + rtx cmp = operands[1]; > + rtx cmp_op1 = operands[2]; > + rtx cmp_op2 = operands[3]; > + rtx move_t = operands[4]; > + rtx move_f = operands[5]; > + rtx mask_reg = operands[6]; > + rtx mask_m1 = CONSTM1_RTX (V2DImode); > + rtx mask_0 = CONST0_RTX (V2DImode); > > - operands[7] = CONSTM1_RTX (V2DImode); > - operands[8] = CONST0_RTX (V2DImode); > -} > - [(set_attr "length" "8") > - (set_attr "type" "vecperm")]) > + if (GET_CODE (mask_reg) == SCRATCH) > + mask_reg = gen_reg_rtx (V2DImode); > > -;; Handle inverting the fpmask comparisons. > -(define_insn_and_split "*movcc_invert_p9" > - [(set (match_operand:SFDF 0 "vsx_register_operand" "=&,") > - (if_then_else:SFDF > - (match_operator:CCFP 1 "invert_fpmask_comparison_operator" > - [(match_operand:SFDF2 2 "vsx_register_operand" ",") > - (match_operand:SFDF2 3 "vsx_register_operand" ",")]) > - (match_operand:SFDF 4 "vsx_register_operand" ",") > - (match_operand:SFDF 5 "vsx_register_operand" ","))) > - (clobber (match_scratch:V2DI 6 "=0,&wa"))] > - "TARGET_P9_MINMAX" > - "#" > - "&& 1" > - [(set (match_dup 6) > - (if_then_else:V2DI (match_dup 9) > - (match_dup 7) > - (match_dup 8))) > - (set (match_dup 0) > - (if_then_else:SFDF (ne (match_dup 6) > - (match_dup 8)) > - (match_dup 5) > - (match_dup 4)))] > -{ > - rtx op1 = operands[1]; > - enum rtx_code cond = reverse_condition_maybe_unordered (GET_CODE (op1)); > + /* Reverse the operands and test if the comparison operator is inverted. */ > + if (invert_fpmask_comparison_operator (cmp, CCFPmode)) > + { > + enum rtx_code cond = reverse_condition_maybe_unordered (GET_CODE (cmp)); > + cmp = gen_rtx_fmt_ee (cond, CCFPmode, cmp_op1, cmp_op2); > + std::swap (move_t, move_f); > + } > > - if (GET_CODE (operands[6]) == SCRATCH) > - operands[6] = gen_reg_rtx (V2DImode); > + /* Emit the compare and set mask instruction. */ > + emit_insn (gen_fpmask (mask_reg, cmp, cmp_op1, cmp_op2, > + mask_m1, mask_0)); > > - operands[7] = CONSTM1_RTX (V2DImode); > - operands[8] = CONST0_RTX (V2DImode); > + /* If we have a 64-bit comparison, but an 128-bit move, we need to extend the > + mask. Because we are using the splat builtin to extend the V2DImode, we > + need to use element 1 on little endian systems. */ > + if (!FLOAT128_IEEE_P (mode) > + && FLOAT128_IEEE_P (mode)) > + { > + rtx element = WORDS_BIG_ENDIAN ? const0_rtx : const1_rtx; > + emit_insn (gen_vsx_xxspltd_v2di (mask_reg, mask_reg, element)); > + } > > - operands[9] = gen_rtx_fmt_ee (cond, CCFPmode, operands[2], operands[3]); > + /* Now emit the XXSEL insn. */ > + emit_insn (gen_xxsel (dest, mask_reg, mask_0, move_t, move_f)); > + DONE; > } > [(set_attr "length" "8") > (set_attr "type" "vecperm")]) > > -(define_insn "*fpmask" > - [(set (match_operand:V2DI 0 "vsx_register_operand" "=wa") > +(define_insn "fpmask" > + [(set (match_operand:V2DI 0 "vsx_register_operand" "=") > (if_then_else:V2DI > (match_operator:CCFP 1 "fpmask_comparison_operator" > - [(match_operand:SFDF 2 "vsx_register_operand" "") > - (match_operand:SFDF 3 "vsx_register_operand" "")]) > + [(match_operand:FSCALAR 2 "vsx_register_operand" "") > + (match_operand:FSCALAR 3 "vsx_register_operand" "")]) > (match_operand:V2DI 4 "all_ones_constant" "") > (match_operand:V2DI 5 "zero_constant" "")))] > "TARGET_P9_MINMAX" > - "xscmp%V1dp %x0,%x2,%x3" > +{ > + return (FLOAT128_IEEE_P (mode) > + ? "xscmp%V1qp %0,%2,%3" > + : "xscmp%V1dp %x0,%x2,%x3"); > +} > [(set_attr "type" "fpcompare")]) > > -(define_insn "*xxsel" > - [(set (match_operand:SFDF 0 "vsx_register_operand" "=") > - (if_then_else:SFDF (ne (match_operand:V2DI 1 "vsx_register_operand" "wa") > - (match_operand:V2DI 2 "zero_constant" "")) > - (match_operand:SFDF 3 "vsx_register_operand" "") > - (match_operand:SFDF 4 "vsx_register_operand" "")))] > +(define_insn "xxsel" > + [(set (match_operand:FSCALAR 0 "vsx_register_operand" "=") > + (if_then_else:FSCALAR > + (ne (match_operand:V2DI 1 "vsx_register_operand" "") > + (match_operand:V2DI 2 "zero_constant" "")) > + (match_operand:FSCALAR 3 "vsx_register_operand" "") > + (match_operand:FSCALAR 4 "vsx_register_operand" "")))] > "TARGET_P9_MINMAX" > "xxsel %x0,%x4,%x3,%x1" > [(set_attr "type" "vecmove")]) > diff --git a/gcc/testsuite/gcc.target/powerpc/float128-cmove.c b/gcc/testsuite/gcc.target/powerpc/float128-cmove.c > new file mode 100644 > index 00000000000..639d5a77087 > --- /dev/null > +++ b/gcc/testsuite/gcc.target/powerpc/float128-cmove.c > @@ -0,0 +1,93 @@ > +/* { dg-do compile } */ > +/* { dg-require-effective-target ppc_float128_hw } */ > +/* { dg-require-effective-target power10_ok } */ > +/* { dg-options "-mdejagnu-cpu=power10 -O2" } */ > +/* { dg-final { scan-assembler {\mxscmpeq[dq]p\M} } } */ > +/* { dg-final { scan-assembler {\mxxpermdi\M} } } */ > +/* { dg-final { scan-assembler {\mxxsel\M} } } */ > +/* { dg-final { scan-assembler-not {\mxscmpu[dq]p\M} } } */ > +/* { dg-final { scan-assembler-not {\mfcmp[uo]\M} } } */ > +/* { dg-final { scan-assembler-not {\mfsel\M} } } */ > + > +/* This series of tests tests whether you can do a conditional move where the > + test is one floating point type, and the result is another floating point > + type. > + > + If the comparison type is SF/DFmode, and the move type is IEEE 128-bit > + floating point, we have to duplicate the mask in the lower 64-bits with > + XXPERMDI because XSCMPEQDP clears the bottom 64-bits of the mask register. > + > + Going the other way (IEEE 128-bit comparsion, 64-bit move) is fine as the > + mask word will be 128-bits. */ > + > +float > +eq_f_d (float a, float b, double x, double y) > +{ > + return (x == y) ? a : b; > +} > + > +double > +eq_d_f (double a, double b, float x, float y) > +{ > + return (x == y) ? a : b; > +} > + > +float > +eq_f_f128 (float a, float b, __float128 x, __float128 y) > +{ > + return (x == y) ? a : b; > +} > + > +double > +eq_d_f128 (double a, double b, __float128 x, __float128 y) > +{ > + return (x == y) ? a : b; > +} > + > +__float128 > +eq_f128_f (__float128 a, __float128 b, float x, float y) > +{ > + return (x == y) ? a : b; > +} > + > +__float128 > +eq_f128_d (__float128 a, __float128 b, double x, double y) > +{ > + return (x != y) ? a : b; > +} > + > +float > +ne_f_d (float a, float b, double x, double y) > +{ > + return (x != y) ? a : b; > +} > + > +double > +ne_d_f (double a, double b, float x, float y) > +{ > + return (x != y) ? a : b; > +} > + > +float > +ne_f_f128 (float a, float b, __float128 x, __float128 y) > +{ > + return (x != y) ? a : b; > +} > + > +double > +ne_d_f128 (double a, double b, __float128 x, __float128 y) > +{ > + return (x != y) ? a : b; > +} > + > +__float128 > +ne_f128_f (__float128 a, __float128 b, float x, float y) > +{ > + return (x != y) ? a : b; > +} > + > +__float128 > +ne_f128_d (__float128 a, __float128 b, double x, double y) > +{ > + return (x != y) ? a : b; > +} > diff --git a/gcc/testsuite/gcc.target/powerpc/float128-minmax-3.c b/gcc/testsuite/gcc.target/powerpc/float128-minmax-3.c > new file mode 100644 > index 00000000000..6f7627c0f2a > --- /dev/null > +++ b/gcc/testsuite/gcc.target/powerpc/float128-minmax-3.c > @@ -0,0 +1,15 @@ > +/* { dg-require-effective-target ppc_float128_hw } */ > +/* { dg-require-effective-target power10_ok } */ > +/* { dg-options "-mdejagnu-cpu=power10 -O2" } */ > + > +#ifndef TYPE > +#define TYPE _Float128 > +#endif > + > +/* Test that the fminf128/fmaxf128 functions generate if/then/else and not a > + call. */ > +TYPE f128_min (TYPE a, TYPE b) { return (a < b) ? a : b; } > +TYPE f128_max (TYPE a, TYPE b) { return (b > a) ? b : a; } > + > +/* { dg-final { scan-assembler {\mxsmaxcqp\M} } } */ > +/* { dg-final { scan-assembler {\mxsmincqp\M} } } */ > -- > 2.22.0 > >