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 B550C3855587 for ; Fri, 21 Jul 2023 23:23:34 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org B550C3855587 Authentication-Results: sourceware.org; dmarc=none (p=none dis=none) header.from=us.ibm.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=us.ibm.com Received: from pps.filterd (m0353728.ppops.net [127.0.0.1]) by mx0a-001b2d01.pphosted.com (8.17.1.19/8.17.1.19) with ESMTP id 36LM6jvQ030387; Fri, 21 Jul 2023 23:23:33 GMT DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=ibm.com; h=message-id : subject : from : to : cc : date : in-reply-to : references : content-type : mime-version : content-transfer-encoding; s=pp1; bh=HRVaQFL4gxnWbiS6AfeDcUOEn4mQ8oyaksZ6QVx7HJk=; b=h9UteM28i055ADd7jxIOjkcX/b0Fu/8knvMGfeh7fBziH7JIkB0ZamZgdRWDkYGWuj+e sEw3AuvPehpeXiBF0Ctl2jF/UPqPzFgGkXS0yHYRQlyOVo1BDGWuc3R9R6fcbnCyzm5N 26NXXKij/hSH6vR7NNxO5tOaRRgC559SuwLa50e5EW6J1bI8ttlyC/QSMeqpcXYfuw/q 3Ep7VGIqHEIdyI/p3WmAfCziwi0Bn3n2Led0PC5oRbzNktHWD5rrjBoTicWcHta3OncU CvJ6kZx+KeNahKlYELDcYiDBkaV1WO62EKErThPWiz7Qkwiv17ubZWbAnsBOXbpy55ac uA== Received: from pps.reinject (localhost [127.0.0.1]) by mx0a-001b2d01.pphosted.com (PPS) with ESMTPS id 3s00pbv3ga-1 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=NOT); Fri, 21 Jul 2023 23:23:32 +0000 Received: from m0353728.ppops.net (m0353728.ppops.net [127.0.0.1]) by pps.reinject (8.17.1.5/8.17.1.5) with ESMTP id 36LNGk3j007100; Fri, 21 Jul 2023 23:23:32 GMT Received: from ppma13.dal12v.mail.ibm.com (dd.9e.1632.ip4.static.sl-reverse.com [50.22.158.221]) by mx0a-001b2d01.pphosted.com (PPS) with ESMTPS id 3s00pbv3g5-1 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=NOT); Fri, 21 Jul 2023 23:23:32 +0000 Received: from pps.filterd (ppma13.dal12v.mail.ibm.com [127.0.0.1]) by ppma13.dal12v.mail.ibm.com (8.17.1.19/8.17.1.19) with ESMTP id 36LMCXht007605; Fri, 21 Jul 2023 23:23:31 GMT Received: from smtprelay02.dal12v.mail.ibm.com ([172.16.1.4]) by ppma13.dal12v.mail.ibm.com (PPS) with ESMTPS id 3rv80jr4qn-1 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=NOT); Fri, 21 Jul 2023 23:23:31 +0000 Received: from smtpav05.dal12v.mail.ibm.com (smtpav05.dal12v.mail.ibm.com [10.241.53.104]) by smtprelay02.dal12v.mail.ibm.com (8.14.9/8.14.9/NCO v10.0) with ESMTP id 36LNNU2M28377694 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-GCM-SHA384 bits=256 verify=OK); Fri, 21 Jul 2023 23:23:30 GMT Received: from smtpav05.dal12v.mail.ibm.com (unknown [127.0.0.1]) by IMSVA (Postfix) with ESMTP id 579D658056; Fri, 21 Jul 2023 23:23:30 +0000 (GMT) Received: from smtpav05.dal12v.mail.ibm.com (unknown [127.0.0.1]) by IMSVA (Postfix) with ESMTP id D124E58052; Fri, 21 Jul 2023 23:23:29 +0000 (GMT) Received: from li-e362e14c-2378-11b2-a85c-87d605f3c641.ibm.com (unknown [9.61.185.149]) by smtpav05.dal12v.mail.ibm.com (Postfix) with ESMTP; Fri, 21 Jul 2023 23:23:29 +0000 (GMT) Message-ID: <4fac537afae5713a7dcc14573bd48b5232371d41.camel@us.ibm.com> Subject: Re: [PATCH 1/2] rs6000, add argument to function find_instance From: Carl Love To: "Kewen.Lin" , cel@us.ibm.com Cc: Peter Bergner , Segher Boessenkool , David Edelsohn , gcc-patches@gcc.gnu.org Date: Fri, 21 Jul 2023 16:23:29 -0700 In-Reply-To: <6828e9a4-a121-0e84-d516-993cc0133eab@linux.ibm.com> References: <949827540816a434c5bac00f0714948638c37975.camel@us.ibm.com> <6828e9a4-a121-0e84-d516-993cc0133eab@linux.ibm.com> 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: IIK8KJuZUZHl1rHmeoVNGkZIP-ABaRVu X-Proofpoint-GUID: 3Uf8dhuEHlSZgB7qvHybpVaAvuQOEtgp 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-07-21_12,2023-07-20_01,2023-05-22_02 X-Proofpoint-Spam-Details: rule=outbound_notspam policy=outbound score=0 lowpriorityscore=0 mlxlogscore=999 mlxscore=0 malwarescore=0 clxscore=1015 impostorscore=0 bulkscore=0 adultscore=0 phishscore=0 priorityscore=1501 spamscore=0 suspectscore=0 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.12.0-2306200000 definitions=main-2307210204 X-Spam-Status: No, score=-10.9 required=5.0 tests=BAYES_00,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_EF,GIT_PATCH_0,RCVD_IN_MSPIKE_H5,RCVD_IN_MSPIKE_WL,SPF_HELO_NONE,SPF_NONE,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 Fri, 2023-07-21 at 10:19 +0800, Kewen.Lin wrote: > Hi Carl, > > on 2023/7/18 03:19, Carl Love wrote: > > GCC maintainers: > > > > The rs6000 function find_instance assumes that it is called for > > built- > > ins with only two arguments. There is no checking for the actual > > number of aruguments used in the built-in. This patch adds an > > additional parameter to the function call containing the number of > > aruguments in the built-in. The function will now do the needed > > checks > > for all of the arguments. > > > > This fix is needed for the next patch in the series that fixes the > > vec_replace_unaligned built-in.c test. > > > > Please let me know if this patch is acceptable for > > mainline. Thanks. > > > > Carl > > > > > > -------------------------------------------- > > rs6000, add argument to function find_instance > > > > The function find_instance assumes it is called to check a built- > > in with Fixed > > ~~ two spaces. > > only two arguments. Ths patch extends the function by adding a > > parameter > s/Ths/This/ > > specifying the number of buit-in arguments to check. > s/bult-in/built-in/ > Fixed both typos. > > gcc/ChangeLog: > > * config/rs6000/rs6000-c.cc (find_instance): Add new parameter > > that > > specifies the number of built-in arguments to check. > > (altivec_resolve_overloaded_builtin): Update calls to > > find_instance > > to pass the number of built-in argument to be checked. > > s/argument/arguments/ fixed > > > --- > > gcc/config/rs6000/rs6000-c.cc | 27 +++++++++++++++++++-------- > > 1 file changed, 19 insertions(+), 8 deletions(-) > > > > diff --git a/gcc/config/rs6000/rs6000-c.cc > > b/gcc/config/rs6000/rs6000-c.cc > > index a353bca19ef..350987b851b 100644 > > --- a/gcc/config/rs6000/rs6000-c.cc > > +++ b/gcc/config/rs6000/rs6000-c.cc > > @@ -1679,7 +1679,7 @@ tree > > There is one function comment here describing the meaning of each > parameter, > I think we should add a corresponding for NARGS, may be something > like: > > "; and NARGS specifies the number of built-in arguments." > Added NARGS description. > Also we need to update the below "two"s with "NARGS". > > "TYPES contains an array of two types..." and "ARGS contains an array > of two arguments..." > Replaced multiple "two" occurrences with NARGS. > since we already extend this to handle NARGS instead of two. > > > find_instance (bool *unsupported_builtin, ovlddata **instance, > > rs6000_gen_builtins instance_code, > > rs6000_gen_builtins fcode, > > - tree *types, tree *args) > > + tree *types, tree *args, int nargs) > > { > > while (*instance && (*instance)->bifid != instance_code) > > *instance = (*instance)->next; > > @@ -1691,17 +1691,28 @@ find_instance (bool *unsupported_builtin, > > ovlddata **instance, > > if (!inst->fntype) > > return error_mark_node; > > tree fntype = rs6000_builtin_info[inst->bifid].fntype; > > - tree parmtype0 = TREE_VALUE (TYPE_ARG_TYPES (fntype)); > > - tree parmtype1 = TREE_VALUE (TREE_CHAIN (TYPE_ARG_TYPES > > (fntype))); > > + tree argtype = TYPE_ARG_TYPES (fntype); > > + tree parmtype; > > Nit: We can move "tree parmtype" into the loop (close to its only > use). Moved and combined declaration with assignment as you noted below. > > > + int args_compatible = true; > > s/int/bool/ Changed. > > > > > - if (rs6000_builtin_type_compatible (types[0], parmtype0) > > - && rs6000_builtin_type_compatible (types[1], parmtype1)) > > + for (int i = 0; i > Nit: formatting issue, space before nargs. > > > { > > + parmtype = TREE_VALUE (argtype); > > tree parmtype = TREE_VALUE (argtype); Changed > > > + if (! rs6000_builtin_type_compatible (types[i], parmtype)) > > Nit: One unexpected(?) space after "!". Removed extra space after "!". > > > + { > > + args_compatible = false; > > + break; > > + } > > + argtype = TREE_CHAIN (argtype); > > + } > > + > > + if (args_compatible) > > + { > > Nit: indent issue for "{". Fixed indent. > > Ok for trunk with these nits fixed. Btw, the description doesn't say > how this was tested, I'm not sure if it's only tested together with > "patch 2/2", but please ensure it's bootstrapped and regress-tested > on BE and LE when committing. Thanks! > Yes, it was tested with patch 2/2 on Power 10 LE. I did do a test on Power 9 as well but don't recall if I tested for both BE and LE. Will retest on Power 8 LE/BE, Power 9 LE/BE and Power 10. Carl