From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mx0b-001b2d01.pphosted.com (mx0b-001b2d01.pphosted.com [148.163.158.5]) by sourceware.org (Postfix) with ESMTPS id B10FC3858D35 for ; Tue, 23 May 2023 05:24:17 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org B10FC3858D35 Authentication-Results: sourceware.org; dmarc=none (p=none dis=none) header.from=linux.ibm.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=linux.ibm.com Received: from pps.filterd (m0353725.ppops.net [127.0.0.1]) by mx0a-001b2d01.pphosted.com (8.17.1.19/8.17.1.19) with ESMTP id 34N4MOdM030416; Tue, 23 May 2023 05:24:17 GMT DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=ibm.com; h=message-id : date : mime-version : subject : to : cc : references : from : in-reply-to : content-type : content-transfer-encoding; s=pp1; bh=gvC8+KO+z5JPkwoIVHrHApq8q1QRoUtzzkHIhGySPmw=; b=p3ia1zYDlrihIC3I+CLCRWwn87OxQKlD+AmjuX4+OcvHY2L9uPHVxH7D4ZTwMxETvmc8 d0HC/VmihuQPtk9BBVEu2i2t9dyvoxjay4j8BfEwo1NPb4FvnNFuI63KsE3Jsh3ecTsr Rx+o+BJxWqrrGkJRcva5VBjJdYTcZvDcJqLuz84tFZ+nMLIIYirdOm6PQgDPvDcJwsRI Ub7Qt7TvVDB1sFnZo9ERQGRKm2MCC8TcjQJQDGPRK+XNN6k3iwgFV4ZHUMnSQCYwjKcn zi4RdPOIVhK47R2cPralfylHRXuowaIykAjb2h+P4gMkYWMLFGODVUAd5hmbAAhTV8+e 2w== Received: from ppma03ams.nl.ibm.com (62.31.33a9.ip4.static.sl-reverse.com [169.51.49.98]) by mx0a-001b2d01.pphosted.com (PPS) with ESMTPS id 3qrn2tu1cq-1 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=NOT); Tue, 23 May 2023 05:24:16 +0000 Received: from pps.filterd (ppma03ams.nl.ibm.com [127.0.0.1]) by ppma03ams.nl.ibm.com (8.17.1.19/8.17.1.19) with ESMTP id 34N2FIkP013811; Tue, 23 May 2023 05:24:14 GMT Received: from smtprelay07.fra02v.mail.ibm.com ([9.218.2.229]) by ppma03ams.nl.ibm.com (PPS) with ESMTPS id 3qppcu9936-1 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=NOT); Tue, 23 May 2023 05:24:14 +0000 Received: from smtpav02.fra02v.mail.ibm.com (smtpav02.fra02v.mail.ibm.com [10.20.54.101]) by smtprelay07.fra02v.mail.ibm.com (8.14.9/8.14.9/NCO v10.0) with ESMTP id 34N5OC0U61538680 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-GCM-SHA384 bits=256 verify=OK); Tue, 23 May 2023 05:24:12 GMT Received: from smtpav02.fra02v.mail.ibm.com (unknown [127.0.0.1]) by IMSVA (Postfix) with ESMTP id 9279420040; Tue, 23 May 2023 05:24:12 +0000 (GMT) Received: from smtpav02.fra02v.mail.ibm.com (unknown [127.0.0.1]) by IMSVA (Postfix) with ESMTP id D056E2004B; Tue, 23 May 2023 05:24:10 +0000 (GMT) Received: from [9.177.28.193] (unknown [9.177.28.193]) by smtpav02.fra02v.mail.ibm.com (Postfix) with ESMTP; Tue, 23 May 2023 05:24:10 +0000 (GMT) Message-ID: Date: Tue, 23 May 2023 13:24:08 +0800 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.15; rv:91.0) Gecko/20100101 Thunderbird/91.6.1 Subject: Re: [PATCH v2] rs6000: Add buildin for mffscrn instructions Content-Language: en-US To: Carl Love Cc: Peter Bergner , Segher Boessenkool , gcc-patches@gcc.gnu.org References: <4f1af7ba04999d0258354b8e6794621ee303ec53.camel@us.ibm.com> From: "Kewen.Lin" In-Reply-To: <4f1af7ba04999d0258354b8e6794621ee303ec53.camel@us.ibm.com> Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit X-TM-AS-GCONF: 00 X-Proofpoint-GUID: MZc403Xh_-vq9Lj3vuBa-_nRtxBfJNMf X-Proofpoint-ORIG-GUID: MZc403Xh_-vq9Lj3vuBa-_nRtxBfJNMf X-Proofpoint-Virus-Version: vendor=baseguard engine=ICAP:2.0.254,Aquarius:18.0.957,Hydra:6.0.573,FMLib:17.11.176.26 definitions=2023-05-23_02,2023-05-22_03,2023-05-22_02 X-Proofpoint-Spam-Details: rule=outbound_notspam policy=outbound score=0 phishscore=0 spamscore=0 impostorscore=0 suspectscore=0 priorityscore=1501 malwarescore=0 bulkscore=0 mlxscore=0 clxscore=1015 mlxlogscore=999 adultscore=0 lowpriorityscore=0 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.12.0-2304280000 definitions=main-2305230040 X-Spam-Status: No, score=-11.8 required=5.0 tests=BAYES_00,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_EF,GIT_PATCH_0,KAM_SHORT,NICE_REPLY_A,RCVD_IN_MSPIKE_H2,SPF_HELO_NONE,SPF_PASS,TXREP,T_SCC_BODY_TEXT_LINE 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 2023/5/23 01:31, Carl Love wrote: > On Mon, 2023-05-22 at 14:36 +0800, Kewen.Lin wrote: >> Hi Carl, >> >> on 2023/5/19 05:12, Carl Love via Gcc-patches wrote: >>> GCC maintainers: >>> >>> version 2. Fixed an issue with the test case. The dg-options line >>> was >>> missing. >>> >>> The following patch adds an overloaded builtin. There are two >>> possible >>> arguments for the builtin. The builtin definitions are: >>> >>> double __builtin_mffscrn (unsigned long int); >>> double __builtin_mffscrn (double); >>> >> >> We already have one bif __builtin_set_fpscr_rn for RN setting, >> apparently >> these two are mainly for direct mapping to mffscr[ni] and want the >> FPSCR bits. >> I'm curious what's the requirements requesting these two built-in >> functions? > > The builtins were requested for use in GLibC. As of version 2.31 they > were added as inline asm. They requested a builtin so the asm could be > removed. > OK, thanks for the information. >> >>> The patch has been tested on Power 10 with no regressions. >>> >>> Please let me know if the patch is acceptable for >>> mainline. Thanks. >>> >>> Carl >>> >>> ------------------------------------------------ >>> rs6000: Add buildin for mffscrn instructions >>> >> >> s/buildin/built-in/ > > fixed >> >>> This patch adds overloaded __builtin_mffscrn for the move From >>> FPSCR >>> Control & Set R instruction with an immediate argument. It also >>> adds the >>> builtin with a floating point register argument. A new runnable >>> test is >>> added for the new builtin. >> >> s/Set R/Set RN/ > > fixed > >>> gcc/ >>> >>> * config/rs6000/rs6000-builtins.def (__builtin_mffscrni, >>> __builtin_mffscrnd): Add builtin definitions. >>> * config/rs6000/rs6000-overload.def (__builtin_mffscrn): Add >>> overloaded definition. >>> * doc/extend.texi: Add documentation for __builtin_mffscrn. >>> >>> gcc/testsuite/ >>> >>> * gcc.target/powerpc/builtin-mffscrn.c: Add testcase for new >>> builtin. >>> --- >>> gcc/config/rs6000/rs6000-builtins.def | 7 ++ >>> gcc/config/rs6000/rs6000-overload.def | 5 + >>> gcc/doc/extend.texi | 8 ++ >>> .../gcc.target/powerpc/builtin-mffscrn.c | 106 >>> ++++++++++++++++++ >>> 4 files changed, 126 insertions(+) >>> create mode 100644 gcc/testsuite/gcc.target/powerpc/builtin- >>> mffscrn.c >>> >>> diff --git a/gcc/config/rs6000/rs6000-builtins.def >>> b/gcc/config/rs6000/rs6000-builtins.def >>> index 92d9b46e1b9..67125473684 100644 >>> --- a/gcc/config/rs6000/rs6000-builtins.def >>> +++ b/gcc/config/rs6000/rs6000-builtins.def >>> @@ -2875,6 +2875,13 @@ >>> pure vsc __builtin_vsx_xl_len_r (void *, signed long); >>> XL_LEN_R xl_len_r {} >>> >>> +; Immediate instruction only uses the least significant two bits >>> of the >>> +; const int. >>> + double __builtin_mffscrni (const int<2>); >>> + MFFSCRNI rs6000_mffscrni {} >>> + >>> + double __builtin_mffscrnd (double); >>> + MFFSCRNF rs6000_mffscrn {} >>> >> >> Why are them put in [power9-64] rather than [power9]? IMHO [power9] >> is the >> correct stanza for them. > > Moved them to power 9 stanza. > >> Besides, {nosoft} attribute is required. > > OK, added that. I was trying to figure out why nosoft is needed. The > instructions are manipulating bits in a physical register that controls > the hardware floating point instructions. It looks to me like that > would be why. Because if you were using msoft float then the floating > point HW registers are disabled and the floating point operations are > done using software. Did I figure this out correctly? Yes, and also the destination of these two instructions is hardware float register, its relatives mffs and mffsl have that as well. > > >> >>> ; Builtins requiring hardware support for IEEE-128 floating-point. >>> [ieee128-hw] >>> diff --git a/gcc/config/rs6000/rs6000-overload.def >>> b/gcc/config/rs6000/rs6000-overload.def >>> index c582490c084..adda2df69ea 100644 >>> --- a/gcc/config/rs6000/rs6000-overload.def >>> +++ b/gcc/config/rs6000/rs6000-overload.def >>> @@ -78,6 +78,11 @@ >>> ; like after a required newline, but nowhere else. Lines >>> beginning with >>> ; a semicolon are also treated as blank lines. >>> >>> +[MFFSCR, __builtin_mffscrn, __builtin_mffscrn] >>> + double __builtin_mffscrn (const int<2>); >>> + MFFSCRNI >>> + double __builtin_mffscrn (double); >>> + MFFSCRNF >>> >>> [BCDADD, __builtin_bcdadd, __builtin_vec_bcdadd] >>> vsq __builtin_vec_bcdadd (vsq, vsq, const int); >>> diff --git a/gcc/doc/extend.texi b/gcc/doc/extend.texi >>> index ed8b9c8a87b..f16c046051a 100644 >>> --- a/gcc/doc/extend.texi >>> +++ b/gcc/doc/extend.texi >>> @@ -18455,6 +18455,9 @@ int __builtin_dfp_dtstsfi_ov_td (unsigned >>> int comparison, _Decimal128 value); >>> >>> double __builtin_mffsl(void); >>> >>> +double __builtin_mffscrn (unsigned long int); >>> +double __builtin_mffscrn (double); >> >> s/unsigned long int/const int/ > > Fixed > >> >> Note that this section is for all configurations and your >> implementation is put >> __builtin_mffscrn power9 only, so if the intention (requirement) is >> to make this >> be for also all configurations, we need to deal with the cases >> without the support >> of actual hw insns mffscrn{,i}, just like the existing handlings for >> mffsl etc. > > I think it should be sufficient to just support these when floating > hardware instructions are supported. So I believe I just need to move > these to the correct place in the document. I think that means they > should be in the section: > > The following functions require @option{-mhard-float}, > @option{-mpowerpc-gfxopt}, and @option{-mpopcntb} options. > > Moved to the end of the above section. Hope that is correct. The hardware insn mffsl which is available starting from ISA 3.0, but the related built-in __builtin_mffsl is available on all configurations, since it has the falling back as rs6000-builtins.def said: ; Although the mffsl instruction is only available on POWER9 and later ; processors, this builtin automatically falls back to mffs on older ; platforms. Thus it appears here in the [always] stanza. double __builtin_mffsl (); MFFSL rs6000_mffsl {nosoft} So IMHO we also want the similar support for mffscrn, that is to make use of mffscrn and mffscrni on Power9 and later, but falls back to __builtin_set_fpscr_rn + mffs similar on older platforms. Segher & Peter, what do you think of this? BR, Kewen