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 5DF463858D35 for ; Thu, 29 Jun 2023 09:13:59 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 5DF463858D35 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 (m0360072.ppops.net [127.0.0.1]) by mx0a-001b2d01.pphosted.com (8.17.1.19/8.17.1.19) with ESMTP id 35T9Cff6001585; Thu, 29 Jun 2023 09:13:58 GMT DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=ibm.com; h=message-id : date : subject : to : cc : references : from : in-reply-to : content-type : content-transfer-encoding : mime-version; s=pp1; bh=0AIeM95q4f4QVKTYxwkOcDVIici9B/JSxRsqGWwa3Zw=; b=ByLKS9h5AMNohOEFEkB+J2idqxbKf7a9Xzs/qYJhG2u+H1vF8gjiJI7sFzXAd615XEjO ADMeFWv68OMdvwd8kiRDDtUgNXFN86PsaLFLfyboocQSoM+h6f2mXOT7v3Wxr0yuaJSb 8CRe+2M1TzA3s4pMkNNcKrhpH8bLJt7Yebha4zvFE3X6GM4RJcn3J/S+ggGTo5sKn7uG X7Z4STikb6ED6XgwP3+cWpk4o9vKmxVaXlaobkeTiAlw7S50kli8NyOfThCndbJHfg1b VdcduPq2kxUJ3DWMbFoYLylnuRG56O3sHr8vY9KRn8/Yg6MBuo3R9+9Zd8kIk0dv8Hym hQ== Received: from pps.reinject (localhost [127.0.0.1]) by mx0a-001b2d01.pphosted.com (PPS) with ESMTPS id 3rh6rare89-1 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=NOT); Thu, 29 Jun 2023 09:13:58 +0000 Received: from m0360072.ppops.net (m0360072.ppops.net [127.0.0.1]) by pps.reinject (8.17.1.5/8.17.1.5) with ESMTP id 35T9D9Q9003682; Thu, 29 Jun 2023 09:13:57 GMT Received: from ppma02fra.de.ibm.com (47.49.7a9f.ip4.static.sl-reverse.com [159.122.73.71]) by mx0a-001b2d01.pphosted.com (PPS) with ESMTPS id 3rh6rare7m-1 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=NOT); Thu, 29 Jun 2023 09:13:57 +0000 Received: from pps.filterd (ppma02fra.de.ibm.com [127.0.0.1]) by ppma02fra.de.ibm.com (8.17.1.19/8.17.1.19) with ESMTP id 35T6TJML013457; Thu, 29 Jun 2023 09:13:55 GMT Received: from smtprelay01.fra02v.mail.ibm.com ([9.218.2.227]) by ppma02fra.de.ibm.com (PPS) with ESMTPS id 3rdr452dxp-1 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=NOT); Thu, 29 Jun 2023 09:13:55 +0000 Received: from smtpav01.fra02v.mail.ibm.com (smtpav01.fra02v.mail.ibm.com [10.20.54.100]) by smtprelay01.fra02v.mail.ibm.com (8.14.9/8.14.9/NCO v10.0) with ESMTP id 35T9Dr0l15008380 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-GCM-SHA384 bits=256 verify=OK); Thu, 29 Jun 2023 09:13:53 GMT Received: from smtpav01.fra02v.mail.ibm.com (unknown [127.0.0.1]) by IMSVA (Postfix) with ESMTP id 1B02D2004D; Thu, 29 Jun 2023 09:13:53 +0000 (GMT) Received: from smtpav01.fra02v.mail.ibm.com (unknown [127.0.0.1]) by IMSVA (Postfix) with ESMTP id 2F60320043; Thu, 29 Jun 2023 09:13:51 +0000 (GMT) Received: from [9.197.225.127] (unknown [9.197.225.127]) by smtpav01.fra02v.mail.ibm.com (Postfix) with ESMTP; Thu, 29 Jun 2023 09:13:50 +0000 (GMT) Message-ID: Date: Thu, 29 Jun 2023 17:13:49 +0800 User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.15; rv:91.0) Gecko/20100101 Thunderbird/91.6.1 Subject: Re: [PATCH] rs6000, __builtin_set_fpscr_rn add retrun value Content-Language: en-US To: Carl Love Cc: Peter Bergner , Segher Boessenkool , dje.gcc@gmail.com, gcc-patches@gcc.gnu.org References: <42d27e659f56f16796c6bfab0799616bbdf6046a.camel@us.ibm.com> From: "Kewen.Lin" In-Reply-To: <42d27e659f56f16796c6bfab0799616bbdf6046a.camel@us.ibm.com> Content-Type: text/plain; charset=UTF-8 X-TM-AS-GCONF: 00 X-Proofpoint-GUID: mP6nIYCU13K037l3SjratF5QTbzKRZJJ X-Proofpoint-ORIG-GUID: b8dmv6pWDJZH271poKhHaaOPEC_TAvTj Content-Transfer-Encoding: 7bit X-Proofpoint-UnRewURL: 0 URL was un-rewritten MIME-Version: 1.0 X-Proofpoint-Virus-Version: vendor=baseguard engine=ICAP:2.0.254,Aquarius:18.0.957,Hydra:6.0.591,FMLib:17.11.176.26 definitions=2023-06-29_01,2023-06-27_01,2023-05-22_02 X-Proofpoint-Spam-Details: rule=outbound_notspam policy=outbound score=0 bulkscore=0 impostorscore=0 clxscore=1015 priorityscore=1501 adultscore=0 phishscore=0 spamscore=0 suspectscore=0 mlxlogscore=999 malwarescore=0 mlxscore=0 lowpriorityscore=0 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.12.0-2305260000 definitions=main-2306290079 X-Spam-Status: No, score=-4.2 required=5.0 tests=BAYES_00,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_EF,KAM_SHORT,NICE_REPLY_A,RCVD_IN_MSPIKE_H5,RCVD_IN_MSPIKE_WL,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: Hi Carl, on 2023/6/19 23:57, Carl Love wrote: > GCC maintainers: > > > The GLibC team requested a builtin to replace the mffscrn and mffscrni inline asm instructions in the GLibC code> Previously there was discussion on adding builtins for the mffscrn instructions. > > https://gcc.gnu.org/pipermail/gcc-patches/2023-May/620261.html > > In the end, it was felt that it would be to extend the existing > __builtin_set_fpscr_rn builtin to return a double instead of a void > type. The desire is that we could have the functionality of the > mffscrn and mffscrni instructions on older ISAs. The two instructions > were initially added in ISA 3.0. The __builtin_set_fpscr_rn has the > needed functionality to set the RN field using the mffscrn and mffscrni > instructions if ISA 3.0 is supported or fall back to using logical > instructions to mask and set the bits for earlier ISAs. The > instructions return the current value of the FPSCR fields DRN, VE, OE, > UE, ZE, XE, NI, RN bit positions then update the RN bit positions with > the new RN value provided. > > The current __builtin_set_fpscr_rn builtin has a return type of void. > So, changing the return type to double and returning the FPSCR fields > DRN, VE, OE, UE, ZE, XE, NI, RN bit positions would then give the > functionally equivalent of the mffscrn and mffscrni instructions. Any > current uses of the builtin would just ignore the return value yet any > new uses could use the return value. So the requirement is for the > change to the __builtin_set_fpscr_rn builtin to be backwardly > compatible and work for all ISAs. > > The following patch changes the return type of the > __builtin_set_fpscr_rn builtin from void to double. The return value > is the current value of the various FPSCR fields DRN, VE, OE, UE, ZE, > XE, NI, RN bit positions when the builtin is called. The builtin then > updated the RN field with the new value provided as an argument to the > builtin. The patch adds new testcases to test_fpscr_rn_builtin.c to > check that the builtin returns the current value of the FPSCR fields > and then updates the RN field. But this patch also introduces one more overloading instance with argument type double, I had a check on glibc usage of mffscrn/mffscrni, I don't think it's necessary to add this new instance, as it takes the given rounding mode as integral type. For examples: #define __fe_mffscrn(rn) \ ({register fenv_union_t __fr; \ if (__builtin_constant_p (rn)) \ __asm__ __volatile__ ( \ ".machine push; .machine \"power9\"; mffscrni %0,%1; .machine pop" \ : "=f" (__fr.fenv) : "n" (rn)); \ else \ { \ __fr.l = (rn); \ __asm__ __volatile__ ( \ ".machine push; .machine \"power9\"; mffscrn %0,%1; .machine pop" \ : "=f" (__fr.fenv) : "f" (__fr.fenv)); \ } \ __fr.fenv; \ }) /* Same as __fesetround_inline, however without runtime check to use DFP mtfsfi syntax (as relax_fenv_state) or if round value is valid. */ static inline void __fesetround_inline_nocheck (const int round) { #ifdef _ARCH_PWR9 __fe_mffscrn (round); #else if (__glibc_likely (GLRO(dl_hwcap2) & PPC_FEATURE2_ARCH_3_00)) __fe_mffscrn (round); else asm volatile ("mtfsfi 7,%0" : : "n" (round)); #endif } So could you just extend return type (from void to double) but without one more overloading instance? Without overloading, we can still use the original bif instance SET_FPSCR_RN and its correpsonding expander rs6000_set_fpscr_rn, just add some more handlings to fetch bits for return value. It would be simpler IMHO. > > The GLibC team has reviewed the patch to make sure it met their needs > as a drop in replacement for the inline asm mffscr and mffscrni > statements in the GLibC code. T > > The patch has been tested on Power 8 LE/BE, Power 9 LE/BE and Power 10 > LE. > > Please let me know if the patch is acceptable for mainline. Thanks. > > Carl > > -------------------------------- > rs6000, __builtin_set_fpscr_rn add retrun value > > Change the return value from void to double. The return value consists of > the FPSCR fields DRN, VE, OE, UE, ZE, XE, NI, RN bit positions. Add an > overloaded version which accepts a double argument. > > The test powerpc/test_fpscr_rn_builtin.c is updated to add tests for the > double reterun value and the new double argument. > > gcc/ChangeLog: > * config/rs6000/rs6000-builtins.def (__builtin_set_fpscr_rn): Delete. > (__builtin_set_fpscr_rn_i): New builtin definition. > (__builtin_set_fpscr_rn_d): New builtin definition. > * config/rs6000/rs6000-overload.def (__builtin_set_fpscr_rn): New > overloaded definition. > * config/rs6000/rs6000.md ((rs6000_get_fpscr_fields): New > define_expand. > (rs6000_update_fpscr_rn_field): New define_expand. > (rs6000_set_fpscr_rn_d): New define expand. > (rs6000_set_fpscr_rn_i): Renamed from rs6000_set_fpscr_rn, Added > return argument. Updated to use new rs6000_get_fpscr_fields and > rs6000_update_fpscr_rn_field define _expands. > * doc/extend.texi (__builtin_set_fpscr_rn): Update description for > the return value and new double argument. > > gcc/testsuite/ChangeLog: > gcc.target/powerpc/test_fpscr_rn_builtin.c: Add new tests th check > double return value. Add tests for overloaded double argument. Since we have the expectation that the existing usage of __builtin_set_fpscr_rn would still work fine, so could we keep the existing one unchanged to test it? and test the new capability with one new test case? BR, Kewen