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 07DC63858D1E for ; Wed, 28 Jun 2023 08:35:51 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 07DC63858D1E 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 (m0356516.ppops.net [127.0.0.1]) by mx0a-001b2d01.pphosted.com (8.17.1.19/8.17.1.19) with ESMTP id 35S8N0rb018963; Wed, 28 Jun 2023 08:35:50 GMT DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=ibm.com; h=message-id : date : mime-version : subject : to : cc : references : from : in-reply-to : content-type : content-transfer-encoding; s=pp1; bh=8M7PZLENntuS7uKhWnBNPY4dxzpunKxORs9MBQnnNmY=; b=jUc8wT9nO6h1HsnPHWlYGlcC7tpLLp1aqCJzrsU72f9Sxgf9y2k6Xofh9yu8k0AtxysA K9FvRvOz/ZQ7MQrkIaYfImjuAJ6zNMwboGZgwhWFnlBIcGT/76T1H+XeEs/YvWgx9h/S W0AzdI9QzvHgU+C0NbCiZbieSse9jR6EBI6cMpfc0Ny/owfhg8a23hAMh2f7Nxp9/KUf 7nKfPseivKhPyOFgOS/1KsUGCpNEnVK83ARv5cQBt5TyubEmOT+fk/bDjafayncetjk2 nDqbO2hWO6Y+Ro9/S5yrhfpeZ22bDAt5BAVgRN+wezc1ogswA+0ZKBW4eSGjeAPSPbGw 8g== Received: from pps.reinject (localhost [127.0.0.1]) by mx0a-001b2d01.pphosted.com (PPS) with ESMTPS id 3rgha807my-1 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=NOT); Wed, 28 Jun 2023 08:35:46 +0000 Received: from m0356516.ppops.net (m0356516.ppops.net [127.0.0.1]) by pps.reinject (8.17.1.5/8.17.1.5) with ESMTP id 35S8PlK6025267; Wed, 28 Jun 2023 08:35:44 GMT Received: from ppma06ams.nl.ibm.com (66.31.33a9.ip4.static.sl-reverse.com [169.51.49.102]) by mx0a-001b2d01.pphosted.com (PPS) with ESMTPS id 3rgha807j0-1 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=NOT); Wed, 28 Jun 2023 08:35:44 +0000 Received: from pps.filterd (ppma06ams.nl.ibm.com [127.0.0.1]) by ppma06ams.nl.ibm.com (8.17.1.19/8.17.1.19) with ESMTP id 35S2h1FT027611; Wed, 28 Jun 2023 08:35:40 GMT Received: from smtprelay01.fra02v.mail.ibm.com ([9.218.2.227]) by ppma06ams.nl.ibm.com (PPS) with ESMTPS id 3rdqre2dxh-1 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=NOT); Wed, 28 Jun 2023 08:35:40 +0000 Received: from smtpav04.fra02v.mail.ibm.com (smtpav04.fra02v.mail.ibm.com [10.20.54.103]) by smtprelay01.fra02v.mail.ibm.com (8.14.9/8.14.9/NCO v10.0) with ESMTP id 35S8Zcnp19333840 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-GCM-SHA384 bits=256 verify=OK); Wed, 28 Jun 2023 08:35:38 GMT Received: from smtpav04.fra02v.mail.ibm.com (unknown [127.0.0.1]) by IMSVA (Postfix) with ESMTP id 7A1AB20040; Wed, 28 Jun 2023 08:35:38 +0000 (GMT) Received: from smtpav04.fra02v.mail.ibm.com (unknown [127.0.0.1]) by IMSVA (Postfix) with ESMTP id 8016020043; Wed, 28 Jun 2023 08:35:36 +0000 (GMT) Received: from [9.197.237.226] (unknown [9.197.237.226]) by smtpav04.fra02v.mail.ibm.com (Postfix) with ESMTP; Wed, 28 Jun 2023 08:35:36 +0000 (GMT) Message-ID: <3f8d0bdc-bddf-d178-ee76-8d41c4b8755f@linux.ibm.com> Date: Wed, 28 Jun 2023 16:35:34 +0800 MIME-Version: 1.0 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: Update the vsx-vector-6.* tests. Content-Language: en-US To: Carl Love Cc: Peter Bergner , Segher Boessenkool , gcc-patches@gcc.gnu.org, David Edelsohn References: <5197d0d8ab5e975ed7e1e928820769e5921f5796.camel@us.ibm.com> <621ac0734ae83c7ca6af00d804a3d3bc2bbbea5b.camel@us.ibm.com> From: "Kewen.Lin" In-Reply-To: <621ac0734ae83c7ca6af00d804a3d3bc2bbbea5b.camel@us.ibm.com> Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit X-TM-AS-GCONF: 00 X-Proofpoint-GUID: -P7KBjhrqkTHi1tjfrF2kUIyN2vyJpVE X-Proofpoint-ORIG-GUID: WKF7G-H40Da4CQ0d78GSCgfLVSzjYYJS 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-28_04,2023-06-27_01,2023-05-22_02 X-Proofpoint-Spam-Details: rule=outbound_notspam policy=outbound score=0 phishscore=0 priorityscore=1501 mlxlogscore=999 adultscore=0 spamscore=0 lowpriorityscore=0 malwarescore=0 impostorscore=0 mlxscore=0 bulkscore=0 clxscore=1015 suspectscore=0 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.12.0-2305260000 definitions=main-2306280075 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,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/22 06:42, Carl Love wrote: > On Mon, 2023-06-19 at 15:17 +0800, Kewen.Lin wrote: >> Hi Carl, >> >> on 2023/5/31 04:46, Carl Love wrote: >>> GCC maintainers: >>> >>> The following patch takes the tests in vsx-vector-6-p7.h, vsx- >>> vector- >>> 6-p8.h, vsx-vector-6-p9.h and reorganizes them into a series of >>> smaller >>> test files by functionality rather than processor version. >>> >>> The patch has been tested on Power 10 with no regressions. >>> >>> Please let me know if this patch is acceptable for >>> mainline. Thanks. >>> >>> Carl >>> >>> ------------------------------------------ >>> rs6000: Update the vsx-vector-6.* tests. >>> >>> The vsx-vector-6.h file is included into the processor specific >>> test files >>> vsx-vector-6.p7.c, vsx-vector-6.p8.c, and vsx-vector-6.p9.c. The >>> .h file >>> contains a large number of vsx vector builtin tests. The processor >>> specific files contain the number of instructions that the tests >>> are >>> expected to generate for that processor. The tests are compile >>> only. >>> >>> The tests are broken up into a seriers of files for related >>> tests. The >>> new tests are runnable tests to verify the builtin argument types >>> and the >> >> But the newly added test cases are all with "dg-do compile", it >> doesn't >> match what you said here. > > Ah, yea, that is wrong. Fixed. > >> >>> functional correctness of each test rather then verifying the type >>> and >>> number of instructions generated. >> >> It's good to have more coverage with runnable case, but we miss some >> test >> coverages on the expected insn counts which cases p{7,8,9}.c can >> provide >> originally. Unless we can ensure it's already tested somewhere else >> (do >> we? it wasn't stated in this patch), I think we still need those >> checks. > > Yea, I was going with a runnable test and didn't include the > instruction counts. Added back in. Rather then doing by processor > version (P8, P9, P10) I was able to do it by BE/LE. The instruction > counts were the same for LE accross processor versions but there are a > few instruction counts that vary with BE and LE. But the original test case only checks for cpu-types (processor version) but not for endianness, it means for the bif usages, there should not be different for endianness. Why does this changes with your new test case? Could you have a further look and make it consistent with some adjustment if possible? As we know, checking insn counts sometimes are fragile, so I think we should try our best to make it as robust as possible in the first place. Besides, the original case also have some differences between p7/p8 and p9. > > I did noticed in one of the tests that the compiler computed the > answers at compile time and thus didn't actually generate the builtin > code. After digging a little more I found a few more tests where the > compiler was doing the calculations and just inserting the answers. I'd suggest you also adding function attribute __attribute__((noipa)) to those functions avoiding the possible inlining. > > So, I moved all of the tests to functions so the compiler would > actually generate the desired builtin code. > >> >>> gcc/testsuite/ >>> * gcc.target/powerpc/vsx-vector-6-1op.c: New test file. >>> * gcc.target/powerpc/vsx-vector-6-2lop.c: New test file. >>> * gcc.target/powerpc/vsx-vector-6-2op.c: New test file. >>> * gcc.target/powerpc/vsx-vector-6-3op.c: New test file. >>> * gcc.target/powerpc/vsx-vector-6-cmp-all.c: New test file. >>> * gcc.target/powerpc/vsx-vector-6-cmp.c: New test file. >>> * gcc.target/powerpc/vsx-vector-6.h: Remove test file. >>> * gcc.target/powerpc/vsx-vector-6-p7.h: Remove test file. >>> * gcc.target/powerpc/vsx-vector-6-p8.h: Remove test file. >>> * gcc.target/powerpc/vsx-vector-6-p9.h: Remove test file. >>> --- >>> .../powerpc/vsx-vector-6-func-1op.c | 319 +++++++++++++ >>> .../powerpc/vsx-vector-6-func-2lop.c | 305 +++++++++++++ >>> .../powerpc/vsx-vector-6-func-2op.c | 278 ++++++++++++ >>> .../powerpc/vsx-vector-6-func-3op.c | 229 ++++++++++ >>> .../powerpc/vsx-vector-6-func-cmp-all.c | 429 >>> ++++++++++++++++++ >>> .../powerpc/vsx-vector-6-func-cmp.c | 237 ++++++++++ >>> .../gcc.target/powerpc/vsx-vector-6.h | 154 ------- >>> .../gcc.target/powerpc/vsx-vector-6.p7.c | 43 -- >>> .../gcc.target/powerpc/vsx-vector-6.p8.c | 43 -- >>> .../gcc.target/powerpc/vsx-vector-6.p9.c | 42 -- >>> 10 files changed, 1797 insertions(+), 282 deletions(-) >>> create mode 100644 gcc/testsuite/gcc.target/powerpc/vsx-vector-6- >>> func-1op.c >>> create mode 100644 gcc/testsuite/gcc.target/powerpc/vsx-vector-6- >>> func-2lop.c >>> create mode 100644 gcc/testsuite/gcc.target/powerpc/vsx-vector-6- >>> func-2op.c >>> create mode 100644 gcc/testsuite/gcc.target/powerpc/vsx-vector-6- >>> func-3op.c >>> create mode 100644 gcc/testsuite/gcc.target/powerpc/vsx-vector-6- >>> func-cmp-all.c >>> create mode 100644 gcc/testsuite/gcc.target/powerpc/vsx-vector-6- >>> func-cmp.c >>> delete mode 100644 gcc/testsuite/gcc.target/powerpc/vsx-vector-6.h >>> delete mode 100644 gcc/testsuite/gcc.target/powerpc/vsx-vector- >>> 6.p7.c >>> delete mode 100644 gcc/testsuite/gcc.target/powerpc/vsx-vector- >>> 6.p8.c >>> delete mode 100644 gcc/testsuite/gcc.target/powerpc/vsx-vector- >>> 6.p9.c >>> >>> diff --git a/gcc/testsuite/gcc.target/powerpc/vsx-vector-6-func- >>> 1op.c b/gcc/testsuite/gcc.target/powerpc/vsx-vector-6-func-1op.c >>> new file mode 100644 >>> index 00000000000..90a360ea158 >>> --- /dev/null >>> +++ b/gcc/testsuite/gcc.target/powerpc/vsx-vector-6-func-1op.c >>> @@ -0,0 +1,319 @@ >>> +/* { dg-do compile { target lp64 } } */ >>> +/* { dg-skip-if "" { powerpc*-*-darwin* } } */ >>> +/* { dg-options "-O2 -mdejagnu-cpu=power7" } */ >>> + >>> +/* Functional test of the one operand vector builtins. */ >>> + >>> +#include >>> +#include >>> +#include >>> + >>> +#define DEBUG 0 >>> + >>> +void abort (void); >>> + >>> +int >>> +main () { >>> + int i; >>> + vector float f_src = { 125.44, 23.04, -338.56, 17.64}; >>> + vector float f_result; >>> + vector float f_abs_expected = { 125.44, 23.04, 338.56, 17.64}; >>> + vector float f_ceil_expected = { 126.0, 24.0, -338, 18.0}; >>> + vector float f_floor_expected = { 125.0, 23.0, -339, 17.0}; >>> + vector float f_nearbyint_expected = { 125.0, 23.0, -339, 18.0}; >>> + vector float f_rint_expected = { 125.0, 23.0, -339, 18.0}; >>> + vector float f_sqrt_expected = { 11.2, 4.8, 18.4, 4.2}; >>> + vector float f_trunc_expected = { 125.0, 23.0, -338, 17}; >>> + >>> + vector double d_src = { 125.44, -338.56}; >>> + vector double d_result; >>> + vector double d_abs_expected = { 125.44, 338.56}; >>> + vector double d_ceil_expected = { 126.0, -338.0}; >>> + vector double d_floor_expected = { 125.0, -339.0}; >>> + vector double d_nearbyint_expected = { 125.0, -339.0}; >>> + vector double d_rint_expected = { 125.0, -339.0}; >>> + vector double d_sqrt_expected = { 11.2, 18.4}; >>> + vector double d_trunc_expected = { 125.0, -338.0}; >>> + >>> + /* Abs, float */ >>> + f_result = vec_abs (f_src); >>> + >>> + if ((f_result[0] != f_abs_expected[0]) >>> + || (f_result[1] != f_abs_expected[1]) >>> + || (f_result[2] != f_abs_expected[2]) >>> + || (f_result[3] != f_abs_expected[3])) >>> +#if DEBUG >>> + { >>> + printf("ERROR: vec_abs (float) expected value does not >>> match\n"); >>> + printf(" expected[0] = %f; result[0] = %f\n", >>> + f_abs_expected[0], f_result[0]); >>> + printf(" expected[1] = %f; result[1] = %f\n", >>> + f_abs_expected[1], f_result[1]); >>> + printf(" expected[2] = %f; result[2] = %f\n", >>> + f_abs_expected[2], f_result[2]); >>> + printf(" expected[3] = %f; result[3] = %f\n", >>> + f_abs_expected[3], f_result[3]); >>> + } >>> +#else >>> + abort(); >>> +#endif >>> + >>> + /* Ceiling, float */ >>> + f_result = vec_ceil (f_src); >>> + >>> + if ((f_result[0] != f_ceil_expected[0]) >>> + || (f_result[1] != f_ceil_expected[1]) >>> + || (f_result[2] != f_ceil_expected[2]) >>> + || (f_result[3] != f_ceil_expected[3])) >>> +#if DEBUG >>> + { >>> + printf("ERROR: vec_ceil (float) expected value does not >>> match\n"); >>> + printf(" expected[0] = %f; result[0] = %f\n", >>> + f_ceil_expected[0], f_result[0]); >>> + printf(" expected[1] = %f; result[1] = %f\n", >>> + f_ceil_expected[1], f_result[1]); >>> + printf(" expected[2] = %f; result[2] = %f\n", >>> + f_ceil_expected[2], f_result[2]); >>> + printf(" expected[3] = %f; result[3] = %f\n", >>> + f_ceil_expected[3], f_result[3]); >>> + } >>> +#else >>> + abort(); >>> +#endif >> >> It looks that you can use some macro for different floating point >> functions >> and its check and dumping here, since the basic skeleton are >> sharable, some >> thing like: >> >> #define >> FLOAT_CHECK(NAME) >> \ >> f_result = >> vec_##NAME(f_src); \ >> >> \ >> if ((f_result[0] != f_##NAME##_expected[0]) >> || \ >> (f_result[1] != f_##NAME##_expected[1]) >> || \ >> (f_result[2] != f_##NAME##_expected[2]) >> || \ >> (f_result[3] != f_##NAME##_expected[3])) >> { \ >> if (DEBUG) >> { \ >> printf("ERROR: vec_%s (float) expected value does not match\n", >> #NAME); \ >> printf(" expected[0] = %f; result[0] = %f\n", >> f_##NAME##_expected[0], \ >> f_result[0]); >> \ >> printf(" expected[1] = %f; result[1] = %f\n", >> f_##NAME##_expected[1], \ >> f_result[1]); >> \ >> printf(" expected[2] = %f; result[2] = %f\n", >> f_##NAME##_expected[2], \ >> f_result[2]); >> \ >> printf(" expected[3] = %f; result[3] = %f\n", >> f_##NAME##_expected[3], \ >> f_result[3]); >> \ >> } >> else >> \ >> abort(); >> \ >> } >> >> ... >> FLOAT_CHECK(ceil) >> FLOAT_CHECK(nearbyint) >> ... >> >> I hope it can help to make it brief and avoid some copy & paste >> typos. >> But since you already expanded them, it's fine to me if you wanted to >> keep >> the expanded ones. :) > > Generally, I am not a big fan of large code macros. But in this case > it is probably a good idea given the repetitive code. When I wrote the > test cases, I had to put comments in to identify were each test started > as it gets lost in the code. Use of a macro makes it much easier to > see what cases are being tested. Yeah, it should help to avoid those typos. Thanks for updating. BR, Kewen