[PATCH] [PR99708] [rs6000] don't expect __ibm128 with 64-bit long double

[PATCH] [PR99708] [rs6000] don't expect __ibm128 with 64-bit long double

[Alexandre Oliva]

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(-)

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;

-- 
Alexandre Oliva, happy hacker                https://FSFLA.org/blogs/lxo/
   Free Software Activist                       GNU Toolchain Engineer
Disinformation flourishes because many people care deeply about injustice
but very few check the facts.  Ask me about <https://stallmansupport.org>

Re: [PATCH] [PR99708] [rs6000] don't expect __ibm128 with 64-bit long double

[Kewen.Lin]

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

Re: [PATCH] [PR99708] [rs6000] don't expect __ibm128 with 64-bit long double

[Alexandre Oliva]

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,

-- 
Alexandre Oliva, happy hacker                https://FSFLA.org/blogs/lxo/
   Free Software Activist                       GNU Toolchain Engineer
Disinformation flourishes because many people care deeply about injustice
but very few check the facts.  Ask me about <https://stallmansupport.org>

Re: [PATCH] [PR99708] [rs6000] don't expect __ibm128 with 64-bit long double

[Kewen.Lin]

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

Re: [PATCH] [PR99708] [rs6000] don't expect __ibm128 with 64-bit long double

[Alexandre Oliva]

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;


-- 
Alexandre Oliva, happy hacker                https://FSFLA.org/blogs/lxo/
   Free Software Activist                       GNU Toolchain Engineer
Disinformation flourishes because many people care deeply about injustice
but very few check the facts.  Ask me about <https://stallmansupport.org>

Re: [PATCH] [PR99708] [rs6000] don't expect __ibm128 with 64-bit long double

[Kewen.Lin]

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

Re: [PATCH] [PR99708] [rs6000] don't expect __ibm128 with 64-bit long double

[Alexandre Oliva]

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 } } */


-- 
Alexandre Oliva, happy hacker                https://FSFLA.org/blogs/lxo/
   Free Software Activist                       GNU Toolchain Engineer
Disinformation flourishes because many people care deeply about injustice
but very few check the facts.  Ask me about <https://stallmansupport.org>