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 0D97F3858D3C for ; Mon, 29 Aug 2022 06:30:00 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 0D97F3858D3C 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 (m0098410.ppops.net [127.0.0.1]) by mx0a-001b2d01.pphosted.com (8.17.1.5/8.17.1.5) with ESMTP id 27T6Kd1X026847; Mon, 29 Aug 2022 06:29:57 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=OkjCDsBUtfLazS9xQbEtg8fII+0U5pPCfNF+grQQKS0=; b=ewgFhA9iEMRQ3LqS7hC2BvRbhwz5TFaAaNww+nfwYah9BxsYoD3L/XETyMI2vZxufCIp TezNMKidf2dEgmERJuPK5+Iy3zhDVteevwghpAfIQT4VguqomOLPLOG4udXmMXbjdIRu 4jN7/B6dHQkg9/YXIFc29MSxEHmZOi9MQfmiasvi/oXECQnFEa0b5e77CeKv9PrP2kYX wAedwBt3jexwV7PEIuu5GXUXLArmolrpvsP6TPyOwXKPnME1Avtk7J7YPcRXJ4lMjAYD Qc05Sbw1qnRndeN4BzUf+BtIp7X4CSeVt4odvKihAV7vyQWrIE/b41lWM5lBUMMbb3Kl Hg== Received: from pps.reinject (localhost [127.0.0.1]) by mx0a-001b2d01.pphosted.com (PPS) with ESMTPS id 3j8r3v04xk-1 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=NOT); Mon, 29 Aug 2022 06:29:57 +0000 Received: from m0098410.ppops.net (m0098410.ppops.net [127.0.0.1]) by pps.reinject (8.17.1.5/8.17.1.5) with ESMTP id 27T6MgkK031282; Mon, 29 Aug 2022 06:29:56 GMT Received: from ppma04ams.nl.ibm.com (63.31.33a9.ip4.static.sl-reverse.com [169.51.49.99]) by mx0a-001b2d01.pphosted.com (PPS) with ESMTPS id 3j8r3v04x1-1 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=NOT); Mon, 29 Aug 2022 06:29:56 +0000 Received: from pps.filterd (ppma04ams.nl.ibm.com [127.0.0.1]) by ppma04ams.nl.ibm.com (8.16.1.2/8.16.1.2) with SMTP id 27T6LeoE001723; Mon, 29 Aug 2022 06:29:54 GMT Received: from b06cxnps3074.portsmouth.uk.ibm.com (d06relay09.portsmouth.uk.ibm.com [9.149.109.194]) by ppma04ams.nl.ibm.com with ESMTP id 3j7aw8swe9-1 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=NOT); Mon, 29 Aug 2022 06:29:54 +0000 Received: from d06av22.portsmouth.uk.ibm.com (d06av22.portsmouth.uk.ibm.com [9.149.105.58]) by b06cxnps3074.portsmouth.uk.ibm.com (8.14.9/8.14.9/NCO v10.0) with ESMTP id 27T6TpJS41026036 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-GCM-SHA384 bits=256 verify=OK); Mon, 29 Aug 2022 06:29:51 GMT Received: from d06av22.portsmouth.uk.ibm.com (unknown [127.0.0.1]) by IMSVA (Postfix) with ESMTP id A80984C044; Mon, 29 Aug 2022 06:29:51 +0000 (GMT) Received: from d06av22.portsmouth.uk.ibm.com (unknown [127.0.0.1]) by IMSVA (Postfix) with ESMTP id 69ADF4C040; Mon, 29 Aug 2022 06:29:50 +0000 (GMT) Received: from [9.197.231.116] (unknown [9.197.231.116]) by d06av22.portsmouth.uk.ibm.com (Postfix) with ESMTP; Mon, 29 Aug 2022 06:29:50 +0000 (GMT) Message-ID: <4df11770-cab2-c057-1c01-031e59145f1c@linux.ibm.com> Date: Mon, 29 Aug 2022 14:29:48 +0800 User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.15; rv:91.0) Gecko/20100101 Thunderbird/91.6.1 Subject: PING^5 [PATCH v3] rs6000: Fix the check of bif argument number [PR104482] Content-Language: en-US To: GCC Patches Cc: David Edelsohn , Segher Boessenkool References: <8a331b83-5dac-53e6-630c-0a03a18662d9@linux.ibm.com> <10f2af43-d178-5416-fdd8-e93e7cbf4df7@linux.ibm.com> <45ea6e52-cf03-e459-5bbf-78cc6d61dad2@linux.ibm.com> From: "Kewen.Lin" In-Reply-To: <45ea6e52-cf03-e459-5bbf-78cc6d61dad2@linux.ibm.com> Content-Type: text/plain; charset=UTF-8 X-TM-AS-GCONF: 00 X-Proofpoint-GUID: 7vo8GHQKW2kWw8E9aU27WJxf--YPv4Mk X-Proofpoint-ORIG-GUID: Vsgrk8vhcmzAzXW9pJNCxb1qxu9Z38uA 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.205,Aquarius:18.0.895,Hydra:6.0.517,FMLib:17.11.122.1 definitions=2022-08-29_03,2022-08-25_01,2022-06-22_01 X-Proofpoint-Spam-Details: rule=outbound_notspam policy=outbound score=0 adultscore=0 lowpriorityscore=0 clxscore=1015 mlxscore=0 impostorscore=0 malwarescore=0 mlxlogscore=999 suspectscore=0 phishscore=0 priorityscore=1501 spamscore=0 bulkscore=0 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.12.0-2207270000 definitions=main-2208290029 X-Spam-Status: No, score=-11.9 required=5.0 tests=BAYES_00,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_EF,GIT_PATCH_0,KAM_SHORT,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: Hi, Gentle ping https://gcc.gnu.org/pipermail/gcc-patches/2022-May/595208.html I think this is a reasonable fix, the behavior is consistent with what we have in the previous built-in framework, I'm going to push this a week later if no objections. :) BR, Kewen >>>>> Hi, >>>>> >>>>> As PR104482 shown, it's one regression about the handlings when >>>>> the argument number is more than the one of built-in function >>>>> prototype. The new bif support only catches the case that the >>>>> argument number is less than the one of function prototype, but >>>>> it misses the case that the argument number is more than the one >>>>> of function prototype. Because it uses "n != expected_args", >>>>> n is updated in >>>>> >>>>> for (n = 0; !VOID_TYPE_P (TREE_VALUE (fnargs)) && n < nargs; >>>>> fnargs = TREE_CHAIN (fnargs), n++) >>>>> >>>>> , it's restricted to be less than or equal to expected_args with >>>>> the guard !VOID_TYPE_P (TREE_VALUE (fnargs)), so it's wrong. >>>>> >>>>> The fix is to use nargs instead, also move the checking hunk's >>>>> location ahead to avoid useless further scanning when the counts >>>>> mismatch. >>>>> >>>>> Bootstrapped and regtested on powerpc64-linux-gnu P8 and >>>>> powerpc64le-linux-gnu P9 and P10. >>>>> >>>>> v3: Update test case with dg-excess-errors. >>>>> >>>>> v2: Add one test case and refine commit logs. >>>>> https://gcc.gnu.org/pipermail/gcc-patches/2022-April/593155.html >>>>> >>>>> v1: https://gcc.gnu.org/pipermail/gcc-patches/2022-March/591768.html >>>>> >>>>> Is it ok for trunk? >>>>> >>>>> BR, >>>>> Kewen >>>>> ----- >>>>> PR target/104482 >>>>> >>>>> gcc/ChangeLog: >>>>> >>>>> * config/rs6000/rs6000-c.cc (altivec_resolve_overloaded_builtin): Fix >>>>> the equality check for argument number, and move this hunk ahead. >>>>> >>>>> gcc/testsuite/ChangeLog: >>>>> >>>>> * gcc.target/powerpc/pr104482.c: New test. >>>>> --- >>>>> gcc/config/rs6000/rs6000-c.cc | 60 ++++++++++----------- >>>>> gcc/testsuite/gcc.target/powerpc/pr104482.c | 16 ++++++ >>>>> 2 files changed, 46 insertions(+), 30 deletions(-) >>>>> create mode 100644 gcc/testsuite/gcc.target/powerpc/pr104482.c >>>>> >>>>> diff --git a/gcc/config/rs6000/rs6000-c.cc b/gcc/config/rs6000/rs6000-c.cc >>>>> index 9c8cbd7a66e..61881f29230 100644 >>>>> --- a/gcc/config/rs6000/rs6000-c.cc >>>>> +++ b/gcc/config/rs6000/rs6000-c.cc >>>>> @@ -1756,6 +1756,36 @@ altivec_resolve_overloaded_builtin (location_t loc, tree fndecl, >>>>> vec *arglist = static_cast *> (passed_arglist); >>>>> unsigned int nargs = vec_safe_length (arglist); >>>>> >>>>> + /* If the number of arguments did not match the prototype, return NULL >>>>> + and the generic code will issue the appropriate error message. Skip >>>>> + this test for functions where we don't fully describe all the possible >>>>> + overload signatures in rs6000-overload.def (because they aren't relevant >>>>> + to the expansion here). If we don't, we get confusing error messages. */ >>>>> + /* As an example, for vec_splats we have: >>>>> + >>>>> +; There are no actual builtins for vec_splats. There is special handling for >>>>> +; this in altivec_resolve_overloaded_builtin in rs6000-c.cc, where the call >>>>> +; is replaced by a constructor. The single overload here causes >>>>> +; __builtin_vec_splats to be registered with the front end so that can happen. >>>>> +[VEC_SPLATS, vec_splats, __builtin_vec_splats] >>>>> + vsi __builtin_vec_splats (vsi); >>>>> + ABS_V4SI SPLATS_FAKERY >>>>> + >>>>> + So even though __builtin_vec_splats accepts all vector types, the >>>>> + infrastructure cheats and just records one prototype. We end up getting >>>>> + an error message that refers to this specific prototype even when we >>>>> + are handling a different argument type. That is completely confusing >>>>> + to the user, so it's best to let these cases be handled individually >>>>> + in the resolve_vec_splats, etc., helper functions. */ >>>>> + >>>>> + if (expected_args != nargs >>>>> + && !(fcode == RS6000_OVLD_VEC_PROMOTE >>>>> + || fcode == RS6000_OVLD_VEC_SPLATS >>>>> + || fcode == RS6000_OVLD_VEC_EXTRACT >>>>> + || fcode == RS6000_OVLD_VEC_INSERT >>>>> + || fcode == RS6000_OVLD_VEC_STEP)) >>>>> + return NULL; >>>>> + >>>>> for (n = 0; >>>>> !VOID_TYPE_P (TREE_VALUE (fnargs)) && n < nargs; >>>>> fnargs = TREE_CHAIN (fnargs), n++) >>>>> @@ -1816,36 +1846,6 @@ altivec_resolve_overloaded_builtin (location_t loc, tree fndecl, >>>>> types[n] = type; >>>>> } >>>>> >>>>> - /* If the number of arguments did not match the prototype, return NULL >>>>> - and the generic code will issue the appropriate error message. Skip >>>>> - this test for functions where we don't fully describe all the possible >>>>> - overload signatures in rs6000-overload.def (because they aren't relevant >>>>> - to the expansion here). If we don't, we get confusing error messages. */ >>>>> - /* As an example, for vec_splats we have: >>>>> - >>>>> -; There are no actual builtins for vec_splats. There is special handling for >>>>> -; this in altivec_resolve_overloaded_builtin in rs6000-c.cc, where the call >>>>> -; is replaced by a constructor. The single overload here causes >>>>> -; __builtin_vec_splats to be registered with the front end so that can happen. >>>>> -[VEC_SPLATS, vec_splats, __builtin_vec_splats] >>>>> - vsi __builtin_vec_splats (vsi); >>>>> - ABS_V4SI SPLATS_FAKERY >>>>> - >>>>> - So even though __builtin_vec_splats accepts all vector types, the >>>>> - infrastructure cheats and just records one prototype. We end up getting >>>>> - an error message that refers to this specific prototype even when we >>>>> - are handling a different argument type. That is completely confusing >>>>> - to the user, so it's best to let these cases be handled individually >>>>> - in the resolve_vec_splats, etc., helper functions. */ >>>>> - >>>>> - if (n != expected_args >>>>> - && !(fcode == RS6000_OVLD_VEC_PROMOTE >>>>> - || fcode == RS6000_OVLD_VEC_SPLATS >>>>> - || fcode == RS6000_OVLD_VEC_EXTRACT >>>>> - || fcode == RS6000_OVLD_VEC_INSERT >>>>> - || fcode == RS6000_OVLD_VEC_STEP)) >>>>> - return NULL; >>>>> - >>>>> /* Some overloads require special handling. */ >>>>> tree returned_expr = NULL; >>>>> resolution res = unresolved; >>>>> diff --git a/gcc/testsuite/gcc.target/powerpc/pr104482.c b/gcc/testsuite/gcc.target/powerpc/pr104482.c >>>>> new file mode 100644 >>>>> index 00000000000..92191265e4c >>>>> --- /dev/null >>>>> +++ b/gcc/testsuite/gcc.target/powerpc/pr104482.c >>>>> @@ -0,0 +1,16 @@ >>>>> +/* { dg-require-effective-target powerpc_vsx_ok } */ >>>>> +/* { dg-options "-mvsx" } */ >>>>> + >>>>> +/* It's to verify no ICE here, ignore error messages about >>>>> + mismatch argument number since they are not test points >>>>> + here. */ >>>>> +/* { dg-excess-errors "pr104482" } */ >>>>> + >>>>> +__attribute__ ((altivec (vector__))) int vsi; >>>>> + >>>>> +double >>>>> +testXXPERMDI (void) >>>>> +{ >>>>> + return __builtin_vsx_xxpermdi (vsi, vsi, 2, 4); >>>>> +} >>>>> + >>>>