From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mx0a-001b2d01.pphosted.com (mx0a-001b2d01.pphosted.com [148.163.156.1]) by sourceware.org (Postfix) with ESMTPS id 55F65394FC18 for ; Wed, 16 Mar 2022 20:06:49 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 55F65394FC18 Received: from pps.filterd (m0098393.ppops.net [127.0.0.1]) by mx0a-001b2d01.pphosted.com (8.16.1.2/8.16.1.2) with SMTP id 22GJmbvT028150; Wed, 16 Mar 2022 20:06:47 GMT Received: from pps.reinject (localhost [127.0.0.1]) by mx0a-001b2d01.pphosted.com with ESMTP id 3eupcmgc3x-1 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=NOT); Wed, 16 Mar 2022 20:06:46 +0000 Received: from m0098393.ppops.net (m0098393.ppops.net [127.0.0.1]) by pps.reinject (8.16.0.43/8.16.0.43) with SMTP id 22GJu9K2024375; Wed, 16 Mar 2022 20:06:46 GMT Received: from ppma04wdc.us.ibm.com (1a.90.2fa9.ip4.static.sl-reverse.com [169.47.144.26]) by mx0a-001b2d01.pphosted.com with ESMTP id 3eupcmgc3j-1 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=NOT); Wed, 16 Mar 2022 20:06:46 +0000 Received: from pps.filterd (ppma04wdc.us.ibm.com [127.0.0.1]) by ppma04wdc.us.ibm.com (8.16.1.2/8.16.1.2) with SMTP id 22GK3Lb8013420; Wed, 16 Mar 2022 20:06:45 GMT Received: from b01cxnp23032.gho.pok.ibm.com (b01cxnp23032.gho.pok.ibm.com [9.57.198.27]) by ppma04wdc.us.ibm.com with ESMTP id 3erk59xtwa-1 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=NOT); Wed, 16 Mar 2022 20:06:45 +0000 Received: from b01ledav003.gho.pok.ibm.com (b01ledav003.gho.pok.ibm.com [9.57.199.108]) by b01cxnp23032.gho.pok.ibm.com (8.14.9/8.14.9/NCO v10.0) with ESMTP id 22GK6i7P21889514 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-GCM-SHA384 bits=256 verify=OK); Wed, 16 Mar 2022 20:06:44 GMT Received: from b01ledav003.gho.pok.ibm.com (unknown [127.0.0.1]) by IMSVA (Postfix) with ESMTP id F3DCEB2067; Wed, 16 Mar 2022 20:06:43 +0000 (GMT) Received: from b01ledav003.gho.pok.ibm.com (unknown [127.0.0.1]) by IMSVA (Postfix) with ESMTP id 1D457B206A; Wed, 16 Mar 2022 20:06:43 +0000 (GMT) Received: from sig-9-65-215-144.ibm.com (unknown [9.65.215.144]) by b01ledav003.gho.pok.ibm.com (Postfix) with ESMTP; Wed, 16 Mar 2022 20:06:42 +0000 (GMT) Message-ID: <670b4207d259cd325822d8bc0c1dac9a892ee765.camel@vnet.ibm.com> Subject: Re: rs6000: RFC/Update support for addg6s instruction. PR100693 From: will schmidt To: Segher Boessenkool Cc: gcc-patches@gcc.gnu.org, David Edelsohn , Peter Bergner Date: Wed, 16 Mar 2022 15:06:42 -0500 In-Reply-To: <20220316181249.GK614@gate.crashing.org> References: <3b4976974ca4a9e481c462ef2b9a4892f1d4174f.camel@vnet.ibm.com> <20220316181249.GK614@gate.crashing.org> Content-Type: text/plain; charset="UTF-8" X-Mailer: Evolution 3.28.5 (3.28.5-18.el8) Mime-Version: 1.0 Content-Transfer-Encoding: 7bit X-TM-AS-GCONF: 00 X-Proofpoint-ORIG-GUID: Yn-sOHpJAJId6HpsXBynY-wbVwiWKY1c X-Proofpoint-GUID: YrIlf-CDq__6eZR5HZW3hhevzs4fESnL X-Proofpoint-Virus-Version: vendor=baseguard engine=ICAP:2.0.205,Aquarius:18.0.850,Hydra:6.0.425,FMLib:17.11.64.514 definitions=2022-03-16_09,2022-03-15_01,2022-02-23_01 X-Proofpoint-Spam-Details: rule=outbound_notspam policy=outbound score=0 impostorscore=0 clxscore=1015 spamscore=0 mlxlogscore=999 suspectscore=0 phishscore=0 lowpriorityscore=0 priorityscore=1501 bulkscore=0 adultscore=0 malwarescore=0 mlxscore=0 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.12.0-2202240000 definitions=main-2203160120 X-Spam-Status: No, score=-5.4 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_EF, KAM_NUMSUBJECT, KAM_SHORT, RCVD_IN_MSPIKE_H5, RCVD_IN_MSPIKE_WL, SPF_HELO_NONE, SPF_PASS, TXREP, T_SCC_BODY_TEXT_LINE autolearn=no autolearn_force=no version=3.4.4 X-Spam-Checker-Version: SpamAssassin 3.4.4 (2020-01-24) 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: Wed, 16 Mar 2022 20:06:51 -0000 On Wed, 2022-03-16 at 13:12 -0500, Segher Boessenkool wrote: > Hi! > > On Wed, Mar 16, 2022 at 12:20:18PM -0500, will schmidt wrote: > > For PR100693, we currently provide an addg6s builtin using unsigned > > int arguments, but we are missing an unsigned long long argument > > equivalent. This patch adds an overload to provide the long long > > version of the builtin. > > > > unsigned long long __builtin_addg6s (unsigned long long, unsigned > > long long); > > > > RFC/concerns: This patch works, but looking briefly at intermediate > > stages > > is not behaving quite as I expected. Looking at the intermediate > > dumps, I > > see in pr100693.original that calls I expect to be routed to the > > internal > > __builtin_addg6s_si() that uses (unsigned int) arguments are > > instead being > > handled by __builtin_addg6s_di() with casts that convert the > > arguments to > > (unsigned long long). > > Did you test with actual 32-bit variables, instead of just function > arguments? Function arguments are always passed in (sign-extended) > registers. > > Like, > > unsigned int f(unsigned int *a, unsigned int *b) > { > return __builtin_addg6s(*a, *b); > } I perhaps missed that subtlety. I'll investigate that further. > > > As a test, I see if I swap the order of the builtins in rs6000- > > overload.def > > I end up with code casting the ULL values to UI, which provides > > truncated > > results, and is similar to what occurs today without this patch. > > > > All that said, this patch seems to work. OK for next stage 1? > > Tested on power8BE as well as LE power8,power9,power10. > > Please ask again when stage 1 has started? > > > gcc/ > > PR target/100693 > > * config/rs6000/rs600-builtins.def: Remove entry for > > __builtin_addgs() > > and add entries for __builtin_addg6s_di() and > > __builtin_addg6s_si(). > > Indent of second and further lines should be at the "*", not two > spaces > after that. > > > - UNSPEC_ADDG6S > > + UNSPEC_ADDG6S_SI > > + UNSPEC_ADDG6S_DI > > You do not need multiple unspec numbers. You can differentiate them > based on the modes of the arguments, already :-) > > > ;; Miscellaneous ISA 2.06 (power7) instructions > > -(define_insn "addg6s" > > +(define_insn "addg6s_si" > > [(set (match_operand:SI 0 "register_operand" "=r") > > (unspec:SI [(match_operand:SI 1 "register_operand" "r") > > (match_operand:SI 2 "register_operand" "r")] > > - UNSPEC_ADDG6S))] > > + UNSPEC_ADDG6S_SI))] > > + "TARGET_POPCNTD" > > + "addg6s %0,%1,%2" > > + [(set_attr "type" "integer")]) > > + > > +(define_insn "addg6s_di" > > + [(set (match_operand:DI 0 "register_operand" "=r") > > + (unspec:DI [(match_operand:DI 1 "register_operand" "r") > > + (match_operand:DI 2 "register_operand" "r")] > > + UNSPEC_ADDG6S_DI))] > > "TARGET_POPCNTD" > > "addg6s %0,%1,%2" > > [(set_attr "type" "integer")]) > > (define_insn "addg6s" > [(set (match_operand:GPR 0 "register_operand" "=r") > (unspec:GPR [(match_operand:GPR 1 "register_operand" "r") > (match_operand:GPR 2 "register_operand" "r")] > UNSPEC_ADDG6S))] > "TARGET_POPCNTD" > "addg6s %0,%1,%2" > [(set_attr "type" "integer")]) > You do not need multiple unspec numbers. You can differentiate them > based on the modes of the arguments, already :-) Yeah, Thats what I thought, which is a big part of why I posted this with RFC. :-) When I attempted this there was an issue with multiple s (behind the GPR predicate) versus the singular "addg6s" define_insn. It's possible I had something else wrong there, but I'll go back to that attempt and work in that direction. > > We do not want DI (here, and in most places) for -m32! > > > --- /dev/null > > +++ b/gcc/testsuite/gcc.target/powerpc/pr100693.c > > @@ -0,0 +1,68 @@ > > +/* { dg-do compile { target { powerpc*-*-linux* } } } */ > > Why only on Linux? > > > +/* { dg-skip-if "" { powerpc*-*-darwin* } } */ > > Why not on Darwin? And why skip it anyway, given the previous line > :-) > > > +/* { dg-require-effective-target powerpc_vsx_ok } */ > > That is the wrong requirement. You want to test for Power7, not for > VSX. I realise you probably copied this from elsewhere :-( (If from > another addg6s testcase, just keep it). Because reasons. :-) The stanzas are copied from the nearby bcd-1.c testcase that has a simpler test for addg6s. Given the input I'll try to correct the stanzas here and limit how much error I carry along. Thanks for the feedback and review. I'll investigate further, and resubmit at stage1. Thanks, -Will > > > Segher