From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mx0a-001b2d01.pphosted.com (mx0b-001b2d01.pphosted.com [148.163.158.5]) by sourceware.org (Postfix) with ESMTPS id 0C7693858433 for ; Thu, 18 Nov 2021 13:32:34 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 0C7693858433 Received: from pps.filterd (m0098413.ppops.net [127.0.0.1]) by mx0b-001b2d01.pphosted.com (8.16.1.2/8.16.1.2) with SMTP id 1AIDHnho011348; Thu, 18 Nov 2021 13:32:32 GMT Received: from pps.reinject (localhost [127.0.0.1]) by mx0b-001b2d01.pphosted.com with ESMTP id 3cdqke8bhy-1 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=NOT); Thu, 18 Nov 2021 13:32:32 +0000 Received: from m0098413.ppops.net (m0098413.ppops.net [127.0.0.1]) by pps.reinject (8.16.0.43/8.16.0.43) with SMTP id 1AIDI3aC011641; Thu, 18 Nov 2021 13:32:32 GMT Received: from ppma02dal.us.ibm.com (a.bd.3ea9.ip4.static.sl-reverse.com [169.62.189.10]) by mx0b-001b2d01.pphosted.com with ESMTP id 3cdqke8bhk-1 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=NOT); Thu, 18 Nov 2021 13:32:32 +0000 Received: from pps.filterd (ppma02dal.us.ibm.com [127.0.0.1]) by ppma02dal.us.ibm.com (8.16.1.2/8.16.1.2) with SMTP id 1AIDJ3KZ027851; Thu, 18 Nov 2021 13:32:31 GMT Received: from b01cxnp22033.gho.pok.ibm.com (b01cxnp22033.gho.pok.ibm.com [9.57.198.23]) by ppma02dal.us.ibm.com with ESMTP id 3ca50dm2xb-1 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=NOT); Thu, 18 Nov 2021 13:32:31 +0000 Received: from b01ledav003.gho.pok.ibm.com (b01ledav003.gho.pok.ibm.com [9.57.199.108]) by b01cxnp22033.gho.pok.ibm.com (8.14.9/8.14.9/NCO v10.0) with ESMTP id 1AIDWTvc11993472 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-GCM-SHA384 bits=256 verify=OK); Thu, 18 Nov 2021 13:32:29 GMT Received: from b01ledav003.gho.pok.ibm.com (unknown [127.0.0.1]) by IMSVA (Postfix) with ESMTP id D2F16B2079; Thu, 18 Nov 2021 13:32:29 +0000 (GMT) Received: from b01ledav003.gho.pok.ibm.com (unknown [127.0.0.1]) by IMSVA (Postfix) with ESMTP id B4A26B2074; Thu, 18 Nov 2021 13:32:28 +0000 (GMT) Received: from [9.211.84.243] (unknown [9.211.84.243]) by b01ledav003.gho.pok.ibm.com (Postfix) with ESMTP; Thu, 18 Nov 2021 13:32:28 +0000 (GMT) Message-ID: <37a1c5e8-a9ca-f5d9-74f3-0a7d82e30a7d@linux.ibm.com> Date: Thu, 18 Nov 2021 07:32:27 -0600 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.15; rv:91.0) Gecko/20100101 Thunderbird/91.3.1 Reply-To: wschmidt@linux.ibm.com Subject: Re: [PATCH] rs6000: Builtins test changes for BFP scalar tests From: Bill Schmidt To: Segher Boessenkool Cc: GCC Patches , David Edelsohn References: <585a2224-e076-9d26-921b-6db56f1606b9@linux.ibm.com> <20211117213236.GV614@gate.crashing.org> <3a4c779c-8476-5619-3632-4feb5c3066b2@linux.ibm.com> In-Reply-To: <3a4c779c-8476-5619-3632-4feb5c3066b2@linux.ibm.com> Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit X-TM-AS-GCONF: 00 X-Proofpoint-GUID: -b8QfUVyrBVsqV1y2Bz_fWfXCsw4-9yn X-Proofpoint-ORIG-GUID: h4u3b2IDbdh8gqU5j5CAgDg6pNCqC0Iu X-Proofpoint-Virus-Version: vendor=baseguard engine=ICAP:2.0.205,Aquarius:18.0.790,Hydra:6.0.425,FMLib:17.0.607.475 definitions=2021-11-18_05,2021-11-17_01,2020-04-07_01 X-Proofpoint-Spam-Details: rule=outbound_notspam policy=outbound score=0 adultscore=0 mlxscore=0 phishscore=0 lowpriorityscore=0 clxscore=1015 bulkscore=0 malwarescore=0 suspectscore=0 mlxlogscore=999 priorityscore=1501 impostorscore=0 spamscore=0 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.12.0-2110150000 definitions=main-2111180077 X-Spam-Status: No, score=-6.6 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_EF, NICE_REPLY_A, RCVD_IN_MSPIKE_H4, RCVD_IN_MSPIKE_WL, SPF_HELO_NONE, SPF_PASS, TXREP autolearn=ham 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: Thu, 18 Nov 2021 13:32:37 -0000 Hi! On 11/17/21 5:06 PM, Bill Schmidt wrote: > On 11/17/21 3:32 PM, Segher Boessenkool wrote: >> On Wed, Nov 17, 2021 at 02:58:54PM -0600, Bill Schmidt wrote: >>> Hi! This patch is broken out of the previous patch for all the builtins test >>> suite adjustments. Here we have some slight changes in error messages due to >>> how the internals have changed between the old and new builtins methods. >>> >>> For scalar-extract-exp-2.c we change: >>> error: '__builtin_vec_scalar_extract_exp is not supported in this compiler configuration' >>> >>> to: >>> error: '__builtin_vsx_scalar_extract_exp' requires the '-mcpu=power9' option and either the '-m64' or '-mpowerpc64' option >>> note: builtin '__builtin_vec_scalar_extract_exp' requires builtin '__builtin_vsx_scalar_extract_exp' >> I don't like that at all. The user didn't write the _vsx thing, and it >> isn't documented either (neither is the _vec one, but that is a separate >> issue, specific to this builtin). > I feel like I haven't explained this well. This kind of thing has been in > existence forever even in the old builtins code. The combination of the > error showing the internal builtin name, and the note tying the overload > name to the internal builtin name, has been there all along. The name of > the internal builtin is pretty meaningless. The only thing that's interesting > in this case is that we previously didn't get this *for this specific case* > because the old code went to a generic fallback. But in many other cases > you get exactly this same kind of error message for the old code. > >>> The new message provides more information. In both cases, it is less than >>> ideal that we don't refer to scalar_extract_exp, which is referenced in >>> the source line, but this is because scalar_extract_exp is #define'd to >>> __builtin_vec_scalar_extract_exp, so it's unavoidable. Certainly this is no >>> worse than before, and arguably better. >> It is a macro, enough said there >> >> The __builtin_ implementation should be documented (in the GCC manual, >> if not elsewhere). The warnings should talk about _vec, because the >> _vsx thing only exists as implementation detail, and we should never >> talk about those. We don't have errors about adddi3 either! >> >>> error: '__builtin_vsx_scalar_extract_sig' requires the '-mcpu=power9' option and either the '-m64' or '-mpowerpc64' option >>> note: builtin '__builtin_vec_scalar_extract_sig' requires builtin '__builtin_vsx_scalar_extract_sig' >> The rhs in the note does not *exist*, as far as the user is concerned. >> One builtin requiring another is all gobbledygook. > As stated above, this isn't something new that I've added. That's what > we already do. It's how the overload error messages have always been. That said, I don't like the "requires" language at all, either. How about replacing "builtin X requires builtin Y" with "overloaded builtin X is implemented by builtin Y" as a better explanation? That could be done easily with a standalone patch that doesn't affect the test suite, since none of the tests check for the note diagnostic. Thanks, Bill > > I haven't been able to eradicate everything awful here... > > Thanks, > Bill > >>> For scalar-test-neg-{2,3,5}.c, we actually change the test case. This is >>> because we deliberately removed some undocumented and pointless overloads, >>> where each overload mapped to a single builtin. These were: >>> __builtin_vec_scalar_test_neg_sp >>> __builtin_vec_scalar_test_neg_dp >>> __builtin_vec_scalar_test_neg_qp >>> which are redundant with the "real" overload: >>> __builtin_vec_scalar_test_neg >>> The latter maps to three builtins of the appropriate type. >> Yes. And the new ones are undocumented and useless just as well, they >> just have better names. >> >> >> Segher