Message ID | orh6u9gray.fsf@lxoliva.fsfla.org |
---|---|
State | New |
Headers |
Return-Path: <gcc-patches-bounces+patchwork=sourceware.org@gcc.gnu.org> X-Original-To: patchwork@sourceware.org Delivered-To: patchwork@sourceware.org Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id 27B1938768A1 for <patchwork@sourceware.org>; Sat, 25 Mar 2023 11:45:35 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org 27B1938768A1 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gcc.gnu.org; s=default; t=1679744735; bh=+V8Bpd7fQdsyXMUxtenRdu1ZByTWpaTgThJ+TWITOo8=; h=To:Cc:Cc:Subject:Date:List-Id:List-Unsubscribe:List-Archive: List-Post:List-Help:List-Subscribe:From:Reply-To:From; b=CbZs2DIv3Dc84iFkDVpT8MKnalZSg5af9lDoAC5s51FkVwnguAk7gGvc1To4FFh2O hjHBz2ZBTgV7rOD9HFhPa+iwbUFigbtiDunWo+9PDndkTUv/enh5mSfJ2amCBgDZus jE6EM5zRbfWNf1JXDbPgxNgK9SmzEf0eGQdRQJTg= X-Original-To: gcc-patches@gcc.gnu.org Delivered-To: gcc-patches@gcc.gnu.org Received: from rock.gnat.com (rock.gnat.com [IPv6:2620:20:4000:0:a9e:1ff:fe9b:1d1]) by sourceware.org (Postfix) with ESMTPS id F160C3858C53; Sat, 25 Mar 2023 11:44:59 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org F160C3858C53 Received: from localhost (localhost.localdomain [127.0.0.1]) by filtered-rock.gnat.com (Postfix) with ESMTP id BE39B1165C5; Sat, 25 Mar 2023 07:44:59 -0400 (EDT) X-Virus-Scanned: Debian amavisd-new at gnat.com X-Amavis-Alert: BAD HEADER SECTION, Duplicate header field: "Cc" Received: from rock.gnat.com ([127.0.0.1]) by localhost (rock.gnat.com [127.0.0.1]) (amavisd-new, port 10024) with LMTP id V2KqS4MyhqUQ; Sat, 25 Mar 2023 07:44:59 -0400 (EDT) Received: from free.home (tron.gnat.com [IPv6:2620:20:4000:0:46a8:42ff:fe0e:e294]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by rock.gnat.com (Postfix) with ESMTPS id 85A6E1165BA; Sat, 25 Mar 2023 07:44:59 -0400 (EDT) Received: from livre (livre.home [172.31.160.2]) by free.home (8.15.2/8.15.2) with ESMTPS id 32P8bPVm1349773 (version=TLSv1.3 cipher=TLS_AES_256_GCM_SHA384 bits=256 verify=NOT); Sat, 25 Mar 2023 05:37:25 -0300 To: gcc-patches@gcc.gnu.org Cc: Rainer Orth <ro@CeBiTec.Uni-Bielefeld.DE>, Mike Stump <mikestump@comcast.net> Cc: David Edelsohn <dje.gcc@gmail.com>, Segher Boessenkool <segher@kernel.crashing.org>, Kewen Lin <linkw@gcc.gnu.org> Subject: [PATCH] [PR99708] [rs6000] don't expect __ibm128 with 64-bit long double Organization: Free thinker, does not speak for AdaCore Date: Sat, 25 Mar 2023 05:37:25 -0300 Message-ID: <orh6u9gray.fsf@lxoliva.fsfla.org> User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/26.3 (gnu/linux) MIME-Version: 1.0 Content-Type: text/plain X-Scanned-By: MIMEDefang 2.84 X-Spam-Status: No, score=-11.2 required=5.0 tests=BAYES_00, DATE_IN_PAST_03_06, GIT_PATCH_0, KAM_DMARC_STATUS, KAM_SHORT, SPF_HELO_NONE, SPF_PASS, TXREP autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) 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 <gcc-patches.gcc.gnu.org> List-Unsubscribe: <https://gcc.gnu.org/mailman/options/gcc-patches>, <mailto:gcc-patches-request@gcc.gnu.org?subject=unsubscribe> List-Archive: <https://gcc.gnu.org/pipermail/gcc-patches/> List-Post: <mailto:gcc-patches@gcc.gnu.org> List-Help: <mailto:gcc-patches-request@gcc.gnu.org?subject=help> List-Subscribe: <https://gcc.gnu.org/mailman/listinfo/gcc-patches>, <mailto:gcc-patches-request@gcc.gnu.org?subject=subscribe> From: Alexandre Oliva via Gcc-patches <gcc-patches@gcc.gnu.org> Reply-To: Alexandre Oliva <oliva@adacore.com> Errors-To: gcc-patches-bounces+patchwork=sourceware.org@gcc.gnu.org Sender: "Gcc-patches" <gcc-patches-bounces+patchwork=sourceware.org@gcc.gnu.org> |
Series |
[PR99708,rs6000] don't expect __ibm128 with 64-bit long double
|
|
Commit Message
Alexandre Oliva
March 25, 2023, 8:37 a.m. UTC
When long double is 64-bit wide, as on vxworks, the rs6000 backend defines neither the __ibm128 type nor the __SIZEOF_IBM128__ macro, but pr99708.c expected both to be always defined. Adjust the test to match the implementation. Regstrapped on ppc64-linux-gnu. Also tested with ppc64-vxworks7r2 (gcc-12). Ok to install? for gcc/testsuite/ChangeLog * gcc.target/powerpc/pr99708.c: Accept lack of __SIZEOF_IBM128__ when long double is 64-bit wide. --- gcc/testsuite/gcc.target/powerpc/pr99708.c | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-)
Comments
Hi Alexandre, Thanks for fixing this. on 2023/3/25 16:37, Alexandre Oliva via Gcc-patches wrote: > > When long double is 64-bit wide, as on vxworks, the rs6000 backend > defines neither the __ibm128 type nor the __SIZEOF_IBM128__ macro, but > pr99708.c expected both to be always defined. Adjust the test to > match the implementation. There is one patch from Mike to define type __ibm128 even without IEEE 128-bit floating point support, it's at the link: https://gcc.gnu.org/pipermail/gcc-patches/2022-August/599984.html I would expect this issue would be gone if the adjustment on the support of type __ibm128 gets landed in future. So maybe we can just xfail this for longdouble64? What do you think? BR, Kewen
Hello, Kewen, Thanks for the feedback. On Mar 27, 2023, "Kewen.Lin" <linkw@linux.ibm.com> wrote: > on 2023/3/25 16:37, Alexandre Oliva via Gcc-patches wrote: >> >> When long double is 64-bit wide, as on vxworks, the rs6000 backend >> defines neither the __ibm128 type nor the __SIZEOF_IBM128__ macro, but >> pr99708.c expected both to be always defined. Adjust the test to >> match the implementation. > There is one patch from Mike to define type __ibm128 even without > IEEE 128-bit floating point support, it's at the link: > https://gcc.gnu.org/pipermail/gcc-patches/2022-August/599984.html > I would expect this issue would be gone if the adjustment on the > support of type __ibm128 gets landed in future. Yeah, the issue would then be gone, but the patch is compatible with that proposed change: if __ibm128 and the corresponding SIZEOF macro are defined, the proposed change is a no-op. > So maybe we can just xfail this for longdouble64? What do you > think? I wouldn't object to that, and I could even write and test the alternate patch for that, but I fail to understand why that would be more desirable. Would you be so kind as to enlighten me? Thanks,
Hi Alexandre, on 2023/4/6 12:43, Alexandre Oliva wrote: > Hello, Kewen, > > Thanks for the feedback. > > On Mar 27, 2023, "Kewen.Lin" <linkw@linux.ibm.com> wrote: > >> on 2023/3/25 16:37, Alexandre Oliva via Gcc-patches wrote: >>> >>> When long double is 64-bit wide, as on vxworks, the rs6000 backend >>> defines neither the __ibm128 type nor the __SIZEOF_IBM128__ macro, but >>> pr99708.c expected both to be always defined. Adjust the test to >>> match the implementation. > >> There is one patch from Mike to define type __ibm128 even without >> IEEE 128-bit floating point support, it's at the link: > >> https://gcc.gnu.org/pipermail/gcc-patches/2022-August/599984.html > >> I would expect this issue would be gone if the adjustment on the >> support of type __ibm128 gets landed in future. > > Yeah, the issue would then be gone, but the patch is compatible with > that proposed change: if __ibm128 and the corresponding SIZEOF macro are > defined, the proposed change is a no-op. Yeah, I agree. > >> So maybe we can just xfail this for longdouble64? What do you >> think? > > I wouldn't object to that, and I could even write and test the alternate > patch for that, but I fail to understand why that would be more > desirable. Would you be so kind as to enlighten me? The reason why personally I preferred to fix it with xfail is that: 1) if the mentioned patch changing the condition of defining __ibm128 type gets re-tested, this case would change from XFAIL to XPASS, then it gets our attentions and shows some benefit of that patch (it can be also mentioned in that commit log). 2) once the mentioned patch gets landed, the below hunk in the proposed patch becomes unreachable: +#else + || __SIZEOF_LONG_DOUBLE__ * __CHAR_BIT__ != 64 +#endif And it's very likely we won't remember to remove it. At that time when someone reads the code, he/she can probably get the impression that type __ibm128 is not always defined even under effective target ppc_float128_sw, it could cause some misunderstanding. Does it make sense to you? BR, Kewen
On Apr 6, 2023, "Kewen.Lin" <linkw@linux.ibm.com> wrote:
> The reason why personally I preferred to fix it with xfail is that:
Got it. I'm convinced, and I agree.
I tried an xfail in the initial dg-do, but that is no good for a compile
error, so I went for a dg-bogus xfail. I hope that will still have the
intended effect when __ibm128 is defined when it currently isn't.
There is a dg-skip-if in this test on the trunk, covering some targets,
that IIRC are longdouble64, so maybe that's related and I could have
dropped them, but I wasn't sure, so I left them alone.
Regstrapped on ppc64-linux-gnu (pass), also tested on ppc64-vx7r2/gcc-12
(xfail). Ok to install?
[PR99708] [rs6000] don't expect __ibm128 with 64-bit long double
When long double is 64-bit wide, as on vxworks, the rs6000 backend
defines neither the __ibm128 type nor the __SIZEOF_IBM128__ macro, but
pr99708.c expected both to be always defined. Adjust the test to
match the implementation.
for gcc/testsuite/ChangeLog
* gcc.target/powerpc/pr99708.c: Accept lack of
__SIZEOF_IBM128__ when long double is 64-bit wide.
---
gcc/testsuite/gcc.target/powerpc/pr99708.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/gcc/testsuite/gcc.target/powerpc/pr99708.c b/gcc/testsuite/gcc.target/powerpc/pr99708.c
index 02b40ebc40d3d..66a5f88479330 100644
--- a/gcc/testsuite/gcc.target/powerpc/pr99708.c
+++ b/gcc/testsuite/gcc.target/powerpc/pr99708.c
@@ -14,7 +14,7 @@
int main (void)
{
if (__SIZEOF_FLOAT128__ != sizeof (__float128)
- || __SIZEOF_IBM128__ != sizeof (__ibm128))
+ || __SIZEOF_IBM128__ != sizeof (__ibm128)) /* { dg-bogus "undeclared" "" { xfail longdouble64 } } */
abort ();
return 0;
Hi Alexandre, on 2023/4/7 09:48, Alexandre Oliva wrote: > On Apr 6, 2023, "Kewen.Lin" <linkw@linux.ibm.com> wrote: > >> The reason why personally I preferred to fix it with xfail is that: > > Got it. I'm convinced, and I agree. > > I tried an xfail in the initial dg-do, but that is no good for a compile > error, so I went for a dg-bogus xfail. I hope that will still have the > intended effect when __ibm128 is defined when it currently isn't. > Thanks for looking into it. > There is a dg-skip-if in this test on the trunk, covering some targets, > that IIRC are longdouble64, so maybe that's related and I could have > dropped them, but I wasn't sure, so I left them alone. I think it's due to that -mfloat128 isn't fully supported on them, so yeah, just leave them alone. > > Regstrapped on ppc64-linux-gnu (pass), also tested on ppc64-vx7r2/gcc-12 > (xfail). Ok to install? > > > [PR99708] [rs6000] don't expect __ibm128 with 64-bit long double > > When long double is 64-bit wide, as on vxworks, the rs6000 backend > defines neither the __ibm128 type nor the __SIZEOF_IBM128__ macro, but > pr99708.c expected both to be always defined. Adjust the test to > match the implementation. > > > for gcc/testsuite/ChangeLog > > * gcc.target/powerpc/pr99708.c: Accept lack of > __SIZEOF_IBM128__ when long double is 64-bit wide. > --- > gcc/testsuite/gcc.target/powerpc/pr99708.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/gcc/testsuite/gcc.target/powerpc/pr99708.c b/gcc/testsuite/gcc.target/powerpc/pr99708.c > index 02b40ebc40d3d..66a5f88479330 100644 > --- a/gcc/testsuite/gcc.target/powerpc/pr99708.c > +++ b/gcc/testsuite/gcc.target/powerpc/pr99708.c > @@ -14,7 +14,7 @@ > int main (void) > { > if (__SIZEOF_FLOAT128__ != sizeof (__float128) > - || __SIZEOF_IBM128__ != sizeof (__ibm128)) > + || __SIZEOF_IBM128__ != sizeof (__ibm128)) /* { dg-bogus "undeclared" "" { xfail longdouble64 } } */ > abort (); > This new version causes unresolved record on my side, it's due to the compilation failed to produce executable. === gcc Summary for unix/-m64 === # of expected passes 1 # of expected failures 1 # of unresolved testcases 1 So I think we need to make the file be compiled well, how about something like: ------ diff --git a/gcc/testsuite/gcc.target/powerpc/pr99708.c b/gcc/testsuite/gcc.target/powerpc/pr99708.c index 02b40ebc40d..c6aa0511b89 100644 --- a/gcc/testsuite/gcc.target/powerpc/pr99708.c +++ b/gcc/testsuite/gcc.target/powerpc/pr99708.c @@ -14,9 +14,17 @@ int main (void) { if (__SIZEOF_FLOAT128__ != sizeof (__float128) - || __SIZEOF_IBM128__ != sizeof (__ibm128)) + /* FIXME: Once type __ibm128 gets supported with long-double-64, + we shouldn't need this conditional #ifdef and xfail. */ +#ifdef __SIZEOF_IBM128__ + || __SIZEOF_IBM128__ != sizeof (__ibm128) +#else + || 1 +#endif + ) abort (); return 0; } +/* { dg-xfail-run-if "unsupported type __ibm128 with long-double-64" { longdouble64 } } */ ------ ? OK if it looks reasonable to you and the testing goes well. Thanks! BR, Kewen
On Apr 7, 2023, "Kewen.Lin" <linkw@linux.ibm.com> wrote: > This new version causes unresolved record on my side, it's due to the > compilation failed to produce executable. Yeah, it does that, I didn't realize that was undesirable. > So I think we need to make the file be compiled well, how about something like: That worked for me as well, so... > ? OK if it looks reasonable to you and the testing goes well. Thanks! ... I'm putting this in, thanks: [PR99708] [rs6000] don't expect __ibm128 with 64-bit long double When long double is 64-bit wide, as on vxworks, the rs6000 backend defines neither the __ibm128 type nor the __SIZEOF_IBM128__ macro, but pr99708.c expected both to be always defined. Adjust the test to match the implementation. Co-Authored-By: Kewen Lin <linkw@linux.ibm.com> for gcc/testsuite/ChangeLog * gcc.target/powerpc/pr99708.c: Accept lack of __SIZEOF_IBM128__ when long double is 64-bit wide. --- gcc/testsuite/gcc.target/powerpc/pr99708.c | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/gcc/testsuite/gcc.target/powerpc/pr99708.c b/gcc/testsuite/gcc.target/powerpc/pr99708.c index 02b40ebc40d3d..c6aa0511b8925 100644 --- a/gcc/testsuite/gcc.target/powerpc/pr99708.c +++ b/gcc/testsuite/gcc.target/powerpc/pr99708.c @@ -14,9 +14,17 @@ int main (void) { if (__SIZEOF_FLOAT128__ != sizeof (__float128) - || __SIZEOF_IBM128__ != sizeof (__ibm128)) + /* FIXME: Once type __ibm128 gets supported with long-double-64, + we shouldn't need this conditional #ifdef and xfail. */ +#ifdef __SIZEOF_IBM128__ + || __SIZEOF_IBM128__ != sizeof (__ibm128) +#else + || 1 +#endif + ) abort (); return 0; } +/* { dg-xfail-run-if "unsupported type __ibm128 with long-double-64" { longdouble64 } } */
diff --git a/gcc/testsuite/gcc.target/powerpc/pr99708.c b/gcc/testsuite/gcc.target/powerpc/pr99708.c index 02b40ebc40d3d..fca6296d9c81f 100644 --- a/gcc/testsuite/gcc.target/powerpc/pr99708.c +++ b/gcc/testsuite/gcc.target/powerpc/pr99708.c @@ -14,7 +14,13 @@ int main (void) { if (__SIZEOF_FLOAT128__ != sizeof (__float128) - || __SIZEOF_IBM128__ != sizeof (__ibm128)) +#ifdef __SIZEOF_IBM128__ /* Not defined, and no __ibm128 type on targets whose + long double is 64-bit. */ + || __SIZEOF_IBM128__ != sizeof (__ibm128) +#else + || __SIZEOF_LONG_DOUBLE__ * __CHAR_BIT__ != 64 +#endif + ) abort (); return 0;