public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
* [PATCH, Fortran] Skip gfortran.dg/PR100914.f90 on targets that don't provide quadmath.h
@ 2021-09-05  2:30 Sandra Loosemore
  2021-09-05 13:31 ` H.J. Lu
  0 siblings, 1 reply; 10+ messages in thread
From: Sandra Loosemore @ 2021-09-05  2:30 UTC (permalink / raw)
  To: fortran, gcc-patches

[-- Attachment #1: Type: text/plain, Size: 874 bytes --]

The testcase gfortran.dg/PR100914.f90 that I recently checked in 
(originally written by José Rui Faustino de Sousa) depends on the 
<quadmath.h> header file to obtain a typedef for __complex128.  It 
appears not to be possible to define an equivalent type in a portable 
way in the testcase itself (see 
https://gcc.gnu.org/onlinedocs/gcc-11.2.0/gcc/Floating-Types.html) so 
this patch skips the test entirely on targets where quadmath.h is not 
available.

The target-supports.exp change was cut-and-pasted from similar code in 
that file, but I haven't figured out how to test this change in a build 
that doesn't provide quadmath.h (e.g., my aarch64-linux-gnu toolchain 
build attempt croaked with an unrelated compilation error in glibc). 
Perhaps someone who previously encountered the FAILs on this testcase 
can confirm that it's skipped with this change?

-Sandra

[-- Attachment #2: pr100914.patch --]
[-- Type: text/x-patch, Size: 1891 bytes --]

commit 41fe3b50b3d92931fc99ef15f86cc9299e0c617e
Author: Sandra Loosemore <sandra@codesourcery.com>
Date:   Sat Sep 4 18:36:39 2021 -0700

    Skip gfortran.dg/PR100914.f90 on targets that don't provide quadmath.h.
    
    This test uses the __complex128 type, which is provided by the
    <quadmath.h> header which may not be available on all targets.
    
    2021-09-04  Sandra Loosemore  <sandra@codesourcery.com>
    
    gcc/testsuite/
    
    	* lib/target-supports.exp (check_effective_target_quadmath_h):
    	New function.
    	* gfortran.dg/PR100914.f90: Use it.  Add comments.

diff --git a/gcc/testsuite/gfortran.dg/PR100914.f90 b/gcc/testsuite/gfortran.dg/PR100914.f90
index 64b3335..aff405a 100644
--- a/gcc/testsuite/gfortran.dg/PR100914.f90
+++ b/gcc/testsuite/gfortran.dg/PR100914.f90
@@ -1,7 +1,10 @@
 ! Fails on x86 targets where sizeof(long double) == 16.
 ! { dg-do run { xfail { { x86_64*-*-* i?86*-*-* } && longdouble128 } } }
-! { dg-additional-sources PR100914.c }
+! Requires Fortran support for __float128.
 ! { dg-require-effective-target fortran_real_c_float128 }
+! Requires __complex128 type from quadmath.h.
+! { dg-require-effective-target quadmath_h }
+! { dg-additional-sources PR100914.c }
 !
 ! Test the fix for PR100914
 ! 
diff --git a/gcc/testsuite/lib/target-supports.exp b/gcc/testsuite/lib/target-supports.exp
index ad8f011..072b776 100644
--- a/gcc/testsuite/lib/target-supports.exp
+++ b/gcc/testsuite/lib/target-supports.exp
@@ -8340,6 +8340,14 @@ proc check_effective_target_libc_has_complex_functions {} {
     }]
 }
 
+# Return true if this target has the quadmath.h header.
+
+proc check_effective_target_quadmath_h {} {
+    return [check_no_compiler_messages quadmath_h object {
+ #include <quadmath.h>
+    }]
+}
+
 # Return 1 if
 #   (a) an error of a few ULP is expected in string to floating-point
 #       conversion functions; and

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH, Fortran] Skip gfortran.dg/PR100914.f90 on targets that don't provide quadmath.h
  2021-09-05  2:30 [PATCH, Fortran] Skip gfortran.dg/PR100914.f90 on targets that don't provide quadmath.h Sandra Loosemore
@ 2021-09-05 13:31 ` H.J. Lu
  2021-09-05 18:02   ` Sandra Loosemore
  0 siblings, 1 reply; 10+ messages in thread
From: H.J. Lu @ 2021-09-05 13:31 UTC (permalink / raw)
  To: Sandra Loosemore; +Cc: fortran, gcc-patches, Christophe Lyon, Sunil K Pandey

[-- Attachment #1: Type: text/plain, Size: 1084 bytes --]

On Sat, Sep 4, 2021 at 7:31 PM Sandra Loosemore <sandra@codesourcery.com> wrote:
>
> The testcase gfortran.dg/PR100914.f90 that I recently checked in
> (originally written by José Rui Faustino de Sousa) depends on the
> <quadmath.h> header file to obtain a typedef for __complex128.  It
> appears not to be possible to define an equivalent type in a portable
> way in the testcase itself (see
> https://gcc.gnu.org/onlinedocs/gcc-11.2.0/gcc/Floating-Types.html) so
> this patch skips the test entirely on targets where quadmath.h is not
> available.
>
> The target-supports.exp change was cut-and-pasted from similar code in
> that file, but I haven't figured out how to test this change in a build
> that doesn't provide quadmath.h (e.g., my aarch64-linux-gnu toolchain
> build attempt croaked with an unrelated compilation error in glibc).
> Perhaps someone who previously encountered the FAILs on this testcase
> can confirm that it's skipped with this change?

Since libqaudmath provides <quadmath.h>, I prefer either of 2 patches
enclosed here.

-- 
H.J.

[-- Attachment #2: p1.diff --]
[-- Type: application/octet-stream, Size: 395 bytes --]

diff --git a/gcc/testsuite/gfortran.dg/PR100914.c b/gcc/testsuite/gfortran.dg/PR100914.c
index c6bd9733e0b..f5db8164eb1 100644
--- a/gcc/testsuite/gfortran.dg/PR100914.c
+++ b/gcc/testsuite/gfortran.dg/PR100914.c
@@ -5,7 +5,7 @@
 #include <stdbool.h>
 #include <stdio.h>
 #include <math.h>
-#include <quadmath.h>
+#include "../../../libquadmath/quadmath.h"
 
 #include <ISO_Fortran_binding.h>
 

[-- Attachment #3: p2.diff --]
[-- Type: application/octet-stream, Size: 863 bytes --]

diff --git a/gcc/testsuite/lib/gfortran.exp b/gcc/testsuite/lib/gfortran.exp
index 43f4d4eda17..50c095cddd2 100644
--- a/gcc/testsuite/lib/gfortran.exp
+++ b/gcc/testsuite/lib/gfortran.exp
@@ -236,6 +236,7 @@ proc gfortran_init { args } {
 #
 
 proc gfortran_target_compile { source dest type options } {
+    global srcdir
     global tmpdir
     global gluefile wrap_flags
     global ALWAYS_GFORTRANFLAGS
@@ -269,6 +270,7 @@ proc gfortran_target_compile { source dest type options } {
     lappend options "compiler=$GFORTRAN_UNDER_TEST"
     lappend options "timeout=[timeout_value]"
 
+    set options [concat "{additional_flags=-I$srcdir/../../libquadmath}" $options]
     set options [concat "$ALWAYS_GFORTRANFLAGS" $options]
     set options [dg-additional-files-options $options $source]
     set return_val [target_compile $source $dest $type $options]

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH, Fortran] Skip gfortran.dg/PR100914.f90 on targets that don't provide quadmath.h
  2021-09-05 13:31 ` H.J. Lu
@ 2021-09-05 18:02   ` Sandra Loosemore
  2021-09-06  1:29     ` H.J. Lu
  0 siblings, 1 reply; 10+ messages in thread
From: Sandra Loosemore @ 2021-09-05 18:02 UTC (permalink / raw)
  To: H.J. Lu; +Cc: fortran, gcc-patches, Christophe Lyon, Sunil K Pandey

On 9/5/21 7:31 AM, H.J. Lu wrote:
> On Sat, Sep 4, 2021 at 7:31 PM Sandra Loosemore <sandra@codesourcery.com> wrote:
>>
>> The testcase gfortran.dg/PR100914.f90 that I recently checked in
>> (originally written by José Rui Faustino de Sousa) depends on the
>> <quadmath.h> header file to obtain a typedef for __complex128.  It
>> appears not to be possible to define an equivalent type in a portable
>> way in the testcase itself (see
>> https://gcc.gnu.org/onlinedocs/gcc-11.2.0/gcc/Floating-Types.html) so
>> this patch skips the test entirely on targets where quadmath.h is not
>> available.
>>
>> The target-supports.exp change was cut-and-pasted from similar code in
>> that file, but I haven't figured out how to test this change in a build
>> that doesn't provide quadmath.h (e.g., my aarch64-linux-gnu toolchain
>> build attempt croaked with an unrelated compilation error in glibc).
>> Perhaps someone who previously encountered the FAILs on this testcase
>> can confirm that it's skipped with this change?
> 
> Since libqaudmath provides <quadmath.h>, I prefer either of 2 patches
> enclosed here.

Of these two, the first one looks better to me, and seems to work OK in 
my x86 build.  But, I'm not sure it is the right thing for ARM/Aarch64 
targets, which apparently have _Float128 support without the __float128 
type or libquadmath.  It's pretty clear quadmath.h depends on having 
__float128.

See Christophe's mail here:

https://gcc.gnu.org/pipermail/fortran/2021-September/056467.html

As I said in my last mail, I ran into some problems getting an aarch64 
toolchain built so I haven't been able to do any testing or experiments 
on that target myself yet.  :-(

-Sandra

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH, Fortran] Skip gfortran.dg/PR100914.f90 on targets that don't provide quadmath.h
  2021-09-05 18:02   ` Sandra Loosemore
@ 2021-09-06  1:29     ` H.J. Lu
  2021-09-06  5:20       ` Sandra Loosemore
  0 siblings, 1 reply; 10+ messages in thread
From: H.J. Lu @ 2021-09-06  1:29 UTC (permalink / raw)
  To: Sandra Loosemore; +Cc: fortran, gcc-patches, Christophe Lyon, Sunil K Pandey

On Sun, Sep 5, 2021 at 11:02 AM Sandra Loosemore
<sandra@codesourcery.com> wrote:
>
> On 9/5/21 7:31 AM, H.J. Lu wrote:
> > On Sat, Sep 4, 2021 at 7:31 PM Sandra Loosemore <sandra@codesourcery.com> wrote:
> >>
> >> The testcase gfortran.dg/PR100914.f90 that I recently checked in
> >> (originally written by José Rui Faustino de Sousa) depends on the
> >> <quadmath.h> header file to obtain a typedef for __complex128.  It
> >> appears not to be possible to define an equivalent type in a portable
> >> way in the testcase itself (see
> >> https://gcc.gnu.org/onlinedocs/gcc-11.2.0/gcc/Floating-Types.html) so
> >> this patch skips the test entirely on targets where quadmath.h is not
> >> available.
> >>
> >> The target-supports.exp change was cut-and-pasted from similar code in
> >> that file, but I haven't figured out how to test this change in a build
> >> that doesn't provide quadmath.h (e.g., my aarch64-linux-gnu toolchain
> >> build attempt croaked with an unrelated compilation error in glibc).
> >> Perhaps someone who previously encountered the FAILs on this testcase
> >> can confirm that it's skipped with this change?
> >
> > Since libqaudmath provides <quadmath.h>, I prefer either of 2 patches
> > enclosed here.
>
> Of these two, the first one looks better to me, and seems to work OK in
> my x86 build.  But, I'm not sure it is the right thing for ARM/Aarch64
> targets, which apparently have _Float128 support without the __float128
> type or libquadmath.  It's pretty clear quadmath.h depends on having
> __float128.

The only <quadmath.h> used by GCC is the one in libquadmath.  I
will check in my first patch tomorrow if there are no objections.

> See Christophe's mail here:
>
> https://gcc.gnu.org/pipermail/fortran/2021-September/056467.html
>
> As I said in my last mail, I ran into some problems getting an aarch64
> toolchain built so I haven't been able to do any testing or experiments
> on that target myself yet.  :-(
>
> -Sandra



-- 
H.J.

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH, Fortran] Skip gfortran.dg/PR100914.f90 on targets that don't provide quadmath.h
  2021-09-06  1:29     ` H.J. Lu
@ 2021-09-06  5:20       ` Sandra Loosemore
  2021-09-06  7:06         ` Christophe Lyon
                           ` (3 more replies)
  0 siblings, 4 replies; 10+ messages in thread
From: Sandra Loosemore @ 2021-09-06  5:20 UTC (permalink / raw)
  To: H.J. Lu; +Cc: fortran, gcc-patches, Christophe Lyon, Sunil K Pandey

On 9/5/21 7:29 PM, H.J. Lu wrote:
> On Sun, Sep 5, 2021 at 11:02 AM Sandra Loosemore
> <sandra@codesourcery.com> wrote:
>>
>> On 9/5/21 7:31 AM, H.J. Lu wrote:
>>> On Sat, Sep 4, 2021 at 7:31 PM Sandra Loosemore <sandra@codesourcery.com> wrote:
>>>>
>>>> The testcase gfortran.dg/PR100914.f90 that I recently checked in
>>>> (originally written by José Rui Faustino de Sousa) depends on the
>>>> <quadmath.h> header file to obtain a typedef for __complex128.  It
>>>> appears not to be possible to define an equivalent type in a portable
>>>> way in the testcase itself (see
>>>> https://gcc.gnu.org/onlinedocs/gcc-11.2.0/gcc/Floating-Types.html) so
>>>> this patch skips the test entirely on targets where quadmath.h is not
>>>> available.
>>>>
>>>> The target-supports.exp change was cut-and-pasted from similar code in
>>>> that file, but I haven't figured out how to test this change in a build
>>>> that doesn't provide quadmath.h (e.g., my aarch64-linux-gnu toolchain
>>>> build attempt croaked with an unrelated compilation error in glibc).
>>>> Perhaps someone who previously encountered the FAILs on this testcase
>>>> can confirm that it's skipped with this change?
>>>
>>> Since libqaudmath provides <quadmath.h>, I prefer either of 2 patches
>>> enclosed here.
>>
>> Of these two, the first one looks better to me, and seems to work OK in
>> my x86 build.  But, I'm not sure it is the right thing for ARM/Aarch64
>> targets, which apparently have _Float128 support without the __float128
>> type or libquadmath.  It's pretty clear quadmath.h depends on having
>> __float128.
> 
> The only <quadmath.h> used by GCC is the one in libquadmath.  I
> will check in my first patch tomorrow if there are no objections.
> 
>> See Christophe's mail here:
>>
>> https://gcc.gnu.org/pipermail/fortran/2021-September/056467.html
>>
>> As I said in my last mail, I ran into some problems getting an aarch64
>> toolchain built so I haven't been able to do any testing or experiments
>> on that target myself yet.  :-(

I finally got an aarch64-linux-gnu toolchain built and confirmed that it 
is still broken there:  it indeed does not define __float128, and 
including quadmath.h results in a gazillion errors like

/path/to/quadmath.h:47:8: error: unknown type name '__float128'

I also observed that _Float128 is the same type as long double on this 
target.

Unless the aarch64 maintainers think it is a bug that __float128 is not 
supported, I think the right solution here is the one I was thinking of 
previously, to fix the Fortran front end to tie the C_FLOAT128 kind to 
_Float128 rather than __float128, and fix the runtime support and test 
cases accordingly.  Then there should be no need to depend on quadmath.h 
at all.  C_FLOAT128 is a GNU extension and _Float128 is supported on a 
superset of targets that __float128 is, so there should be no issue with 
backward compatibility.

-Sandra

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH, Fortran] Skip gfortran.dg/PR100914.f90 on targets that don't provide quadmath.h
  2021-09-06  5:20       ` Sandra Loosemore
@ 2021-09-06  7:06         ` Christophe Lyon
  2021-09-06 10:18         ` Tobias Burnus
                           ` (2 subsequent siblings)
  3 siblings, 0 replies; 10+ messages in thread
From: Christophe Lyon @ 2021-09-06  7:06 UTC (permalink / raw)
  To: Sandra Loosemore; +Cc: H.J. Lu, fortran, gcc-patches, Sunil K Pandey

On Mon, Sep 6, 2021 at 7:21 AM Sandra Loosemore <sandra@codesourcery.com>
wrote:

> On 9/5/21 7:29 PM, H.J. Lu wrote:
> > On Sun, Sep 5, 2021 at 11:02 AM Sandra Loosemore
> > <sandra@codesourcery.com> wrote:
> >>
> >> On 9/5/21 7:31 AM, H.J. Lu wrote:
> >>> On Sat, Sep 4, 2021 at 7:31 PM Sandra Loosemore <
> sandra@codesourcery.com> wrote:
> >>>>
> >>>> The testcase gfortran.dg/PR100914.f90 that I recently checked in
> >>>> (originally written by José Rui Faustino de Sousa) depends on the
> >>>> <quadmath.h> header file to obtain a typedef for __complex128.  It
> >>>> appears not to be possible to define an equivalent type in a portable
> >>>> way in the testcase itself (see
> >>>> https://gcc.gnu.org/onlinedocs/gcc-11.2.0/gcc/Floating-Types.html) so
> >>>> this patch skips the test entirely on targets where quadmath.h is not
> >>>> available.
> >>>>
> >>>> The target-supports.exp change was cut-and-pasted from similar code in
> >>>> that file, but I haven't figured out how to test this change in a
> build
> >>>> that doesn't provide quadmath.h (e.g., my aarch64-linux-gnu toolchain
> >>>> build attempt croaked with an unrelated compilation error in glibc).
> >>>> Perhaps someone who previously encountered the FAILs on this testcase
> >>>> can confirm that it's skipped with this change?
> >>>
> >>> Since libqaudmath provides <quadmath.h>, I prefer either of 2 patches
> >>> enclosed here.
> >>
> >> Of these two, the first one looks better to me, and seems to work OK in
> >> my x86 build.  But, I'm not sure it is the right thing for ARM/Aarch64
> >> targets, which apparently have _Float128 support without the __float128
> >> type or libquadmath.  It's pretty clear quadmath.h depends on having
> >> __float128.
> >
> > The only <quadmath.h> used by GCC is the one in libquadmath.  I
> > will check in my first patch tomorrow if there are no objections.
> >
> >> See Christophe's mail here:
> >>
> >> https://gcc.gnu.org/pipermail/fortran/2021-September/056467.html
> >>
> >> As I said in my last mail, I ran into some problems getting an aarch64
> >> toolchain built so I haven't been able to do any testing or experiments
> >> on that target myself yet.  :-(
>
> I finally got an aarch64-linux-gnu toolchain built and confirmed that it
> is still broken there:  it indeed does not define __float128, and
> including quadmath.h results in a gazillion errors like
>
> /path/to/quadmath.h:47:8: error: unknown type name '__float128'
>
> I also observed that _Float128 is the same type as long double on this
> target.
>
> Unless the aarch64 maintainers think it is a bug that __float128 is not
> supported, I think the right solution here is the one I was thinking of
> previously, to fix the Fortran front end to tie the C_FLOAT128 kind to
> _Float128 rather than __float128, and fix the runtime support and test
> cases accordingly.  Then there should be no need to depend on quadmath.h
> at all.  C_FLOAT128 is a GNU extension and _Float128 is supported on a
> superset of targets that __float128 is, so there should be no issue with
> backward compatibility.
>
>
I'm not really in a position to make a comment, but any patch aiming at
skipping pr100914.f90 only will have no effect on the other errors I
reported, which are related to __float128 vs _Float128.

Christophe


> -Sandra
>

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH, Fortran] Skip gfortran.dg/PR100914.f90 on targets that don't provide quadmath.h
  2021-09-06  5:20       ` Sandra Loosemore
  2021-09-06  7:06         ` Christophe Lyon
@ 2021-09-06 10:18         ` Tobias Burnus
  2021-09-06 16:31         ` Joseph Myers
  2021-09-17  4:02         ` [PATCH, Fortran] Use _Float128 rather than __float128 for c_float128 kind Sandra Loosemore
  3 siblings, 0 replies; 10+ messages in thread
From: Tobias Burnus @ 2021-09-06 10:18 UTC (permalink / raw)
  To: Sandra Loosemore, H.J. Lu
  Cc: Sunil K Pandey, Christophe Lyon, gcc-patches, fortran

Hi Sandra, hi all,

On 06.09.21 07:20, Sandra Loosemore wrote:
> On 9/5/21 7:29 PM, H.J. Lu wrote:
>> On Sun, Sep 5, 2021 at 11:02 AM Sandra Loosemore
>> <sandra@codesourcery.com> wrote:
>>>
>>> On 9/5/21 7:31 AM, H.J. Lu wrote:
>>>> On Sat, Sep 4, 2021 at 7:31 PM Sandra Loosemore
>>>> <sandra@codesourcery.com> wrote:
>>>>>
>>>>> The testcase gfortran.dg/PR100914.f90 that I recently checked in
>>>>> (originally written by José Rui Faustino de Sousa) depends on the
>>>>> <quadmath.h> header file to obtain a typedef for __complex128.
>>>>> https://gcc.gnu.org/onlinedocs/gcc-11.2.0/gcc/Floating-Types.html) so
>>>>> this patch skips the test entirely on targets where quadmath.h is not
>>>>> available.
>>>>>
Can't we just use the following in the testcase?

typedef _Complex _Float128 __complex128;

That avoids the including quadmath.h – and my impression is
that the only reason for including quadmath.h is this single
function. Or does this run into issues with the proper mode?

At least when I tried it, it did compile on x86-64. On the
other hand, there is the following remark in the header file:

/* Define the complex type corresponding to __float128
    ("_Complex __float128" is not allowed) */
#if (!defined(_ARCH_PPC)) || defined(__LONG_DOUBLE_IEEE128__)
typedef _Complex float __attribute__((mode(TC))) __complex128;
#else
typedef _Complex float __attribute__((mode(KC))) __complex128;
#endif

The '#else' was added with
https://gcc.gnu.org/g:0c949f0a1ce9cfa8c48e62628493140d60e65ea7
for https://gcc.gnu.org/PR81848

The comment about _Complex __float128 was in the original patch.

>>> See Christophe's mail here:
>>> https://gcc.gnu.org/pipermail/fortran/2021-September/056467.html
>>>
>>> As I said in my last mail, I ran into some problems getting an aarch64
>>> toolchain built so I haven't been able to do any testing or experiments
>>> on that target myself yet.  :-(
> I finally got an aarch64-linux-gnu toolchain built and confirmed that
> it is still broken there:  it indeed does not define __float128, and
> including quadmath.h results in a gazillion errors like
>
> /path/to/quadmath.h:47:8: error: unknown type name '__float128'
>
> I also observed that _Float128 is the same type as long double on this
> target.

Which means that it is pointless to provide libquadmath on this target
as as the libc's long double math functions are sufficient. – Or not, I
recall that that on PowerPC libquadmath has a effective higher precision
than libc's long double. But in terms of checking type =

> Unless the aarch64 maintainers think it is a bug that __float128 is
> not supported, I think the right solution here is the one I was
> thinking of previously, to fix the Fortran front end to tie the
> C_FLOAT128 kind to _Float128 rather than __float128, and fix the
> runtime support and test cases accordingly.
Whorks for me – I assume that includes updating the documentation. As on
many systems __float128 and _Float128 is available, I don't think it has
to be mentioned in the release notes (but it can).

Tobias

-----------------
Siemens Electronic Design Automation GmbH; Anschrift: Arnulfstraße 201, 80634 München; Gesellschaft mit beschränkter Haftung; Geschäftsführer: Thomas Heurung, Frank Thürauf; Sitz der Gesellschaft: München; Registergericht München, HRB 106955

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH, Fortran] Skip gfortran.dg/PR100914.f90 on targets that don't provide quadmath.h
  2021-09-06  5:20       ` Sandra Loosemore
  2021-09-06  7:06         ` Christophe Lyon
  2021-09-06 10:18         ` Tobias Burnus
@ 2021-09-06 16:31         ` Joseph Myers
  2021-09-17  4:02         ` [PATCH, Fortran] Use _Float128 rather than __float128 for c_float128 kind Sandra Loosemore
  3 siblings, 0 replies; 10+ messages in thread
From: Joseph Myers @ 2021-09-06 16:31 UTC (permalink / raw)
  To: Sandra Loosemore; +Cc: H.J. Lu, gcc-patches, fortran

On Sun, 5 Sep 2021, Sandra Loosemore wrote:

> Unless the aarch64 maintainers think it is a bug that __float128 is not
> supported, I think the right solution here is the one I was thinking of
> previously, to fix the Fortran front end to tie the C_FLOAT128 kind to
> _Float128 rather than __float128, and fix the runtime support and test cases
> accordingly.  Then there should be no need to depend on quadmath.h at all.
> C_FLOAT128 is a GNU extension and _Float128 is supported on a superset of
> targets that __float128 is, so there should be no issue with backward
> compatibility.

gfc_build_intrinsic_lib_fndecls (at least) knows about the "q" function 
naming in libquadmath, and I think will result in gfortran generating 
calls to *q functions in some cases.


I think there are at least three cases involved:


(a) long double has the IEEE binary128 format.  In that case, the *l 
functions from libm can be used, with no need for libquadmath to be 
involved at all.  (It's still necessary to allow for the long double libm 
functions possibly having a different assembler name, for the powerpc64le 
case, but if the front end goes via built-in functions then it may not 
need further code to handle that specifically; see 
rs6000_mangle_decl_assembler_name.)


(b) long double does not have the IEEE binary128 format, but glibc 
includes support for _Float128 functions (*f128) in libm for this target.  
Whether that support is included depends on the architecture and glibc 
version (glibc 2.26 or later needed; supported for 
x86_64/x86/ia64/powerpc64le).  If the glibc support is present (could be 
tested using GCC_GLIBC_VERSION_GTE_IFELSE in configure to work properly 
when bootstrapping a cross compiler), it would make sense for the Fortran 
front end to use *f128 functions directly and so not depend on 
libquadmath.  But I don't think the Fortran front end actually has any 
support for using *f128 functions at present.

It's possible some non-glibc libraries also support *f128 or will do so in 
future (those are the standard names in TS 18661-3 / C23).  However, you 
can't do configure tests of compiling/linking with target libraries when 
configuring front ends, only grep headers or use information about the 
configured target triplet or configure options (which is more or less what 
GCC_GLIBC_VERSION_GTE_IFELSE does), so I don't think there's anything that 
could readily be done to allow using *f128 from non-glibc libm 
automatically if such libm gains support for those functions in future.


(c) __float128 is supported, different format from long double, but glibc 
does not have _Float128 functions (too old) or a non-glibc libm is in use.  
This is the only case where libquadmath is actually relevant.

(libquadmath code is based on glibc, with some support 
(update-quadmath.py) for updating parts of that code from glibc sources 
automatically (most of the parts based on glibc libm, but not the string 
conversion parts).  In practice, it's likely that changes to 
update-quadmath.py would be needed as part of such an update.)


-- 
Joseph S. Myers
joseph@codesourcery.com

^ permalink raw reply	[flat|nested] 10+ messages in thread

* [PATCH, Fortran] Use _Float128 rather than __float128 for c_float128 kind
  2021-09-06  5:20       ` Sandra Loosemore
                           ` (2 preceding siblings ...)
  2021-09-06 16:31         ` Joseph Myers
@ 2021-09-17  4:02         ` Sandra Loosemore
  2021-09-17  8:27           ` Tobias Burnus
  3 siblings, 1 reply; 10+ messages in thread
From: Sandra Loosemore @ 2021-09-17  4:02 UTC (permalink / raw)
  To: fortran, gcc-patches

[-- Attachment #1: Type: text/plain, Size: 837 bytes --]

On 9/5/21 11:20 PM, Sandra Loosemore wrote:

> Unless the aarch64 maintainers think it is a bug that __float128 is not 
> supported, I think the right solution here is the one I was thinking of 
> previously, to fix the Fortran front end to tie the C_FLOAT128 kind to 
> _Float128 rather than __float128, and fix the runtime support and test 
> cases accordingly.  Then there should be no need to depend on quadmath.h 
> at all.  C_FLOAT128 is a GNU extension and _Float128 is supported on a 
> superset of targets that __float128 is, so there should be no issue with 
> backward compatibility.

Here's a new patch that does this.  I've tested it on x86_64-linux-gnu, 
powerpc64le-linux-gnu, and aarch64-linux-gnu, and it does fix the 
previously reported failure compiling gfortran.dg/PR100914.c on aarch64. 
  OK to commit?

-Sandra

[-- Attachment #2: float128.patch --]
[-- Type: text/x-patch, Size: 13629 bytes --]

commit cc7e47df550485654efa5f523c3be35007125340
Author: Sandra Loosemore <sandra@codesourcery.com>
Date:   Tue Sep 14 19:07:36 2021 -0700

    Fortran: Use _Float128 rather than __float128 for c_float128 kind.
    
    The GNU Fortran manual documents that the c_float128 kind corresponds
    to __float128, but in fact the implementation uses float128_type_node,
    which is _Float128.  Both refer to the 128-bit IEEE/ISO encoding, but
    some targets including aarch64 only define _Float128 and not __float128,
    and do not provide quadmath.h.  This caused errors in some test cases
    referring to __float128.
    
    This patch changes the documentation (including code comments) and
    test cases to use _Float128 to match the implementation.
    
    2021-09-16  Sandra Loosemore  <sandra@codesourcery.com>
    
    gcc/fortran/
    
    	* intrinsic.texi (ISO_C_BINDING): Change C_FLOAT128 to correspond
    	to _Float128 rather than __float128.
    	* iso-c-binding.def (c_float128): Update comments.
    	* trans-intrinsic.c (gfc_builtin_decl_for_float_kind): Likewise.
    	(build_round_expr): Likewise.
    	(gfc_build_intrinsic_lib_fndcecls): Likewise.
    	* trans-types.h (gfc_real16_is_float128): Likewise.
    
    gcc/testsuite/
    	* gfortran.dg/PR100914.c: Do not include quadmath.h.  Use
    	_Float128 _Complex instead of __complex128.
    	* gfortran.dg/PR100914.f90: Add -Wno-pedantic to suppress error
    	about use of _Float128.
    	* gfortran.dg/c-interop/typecodes-array-float128-c.c: Use
    	_Float128 instead of __float128.
    	* gfortran.dg/c-interop/typecodes-sanity-c.c: Likewise.
    	* gfortran.dg/c-interop/typecodes-scalar-float128-c.c: Likewise.
    	* lib/target-supports.exp
    	(check_effective_target_fortran_real_c_float128): Update comments.
    
    libgfortran/
    	* ISO_Fortran_binding.h: Update comments.
    	* runtime/ISO_Fortran_binding.c: Likewise.

diff --git a/gcc/fortran/intrinsic.texi b/gcc/fortran/intrinsic.texi
index 1aacd33..1b9a89d 100644
--- a/gcc/fortran/intrinsic.texi
+++ b/gcc/fortran/intrinsic.texi
@@ -15193,8 +15193,8 @@ In addition to the integer named constants required by the Fortran 2003
 standard and @code{C_PTRDIFF_T} of TS 29113, GNU Fortran provides as an
 extension named constants for the 128-bit integer types supported by the
 C compiler: @code{C_INT128_T, C_INT_LEAST128_T, C_INT_FAST128_T}.
-Furthermore, if @code{__float128} is supported in C, the named constants
-@code{C_FLOAT128, C_FLOAT128_COMPLEX} are defined.
+Furthermore, if @code{_Float128} is supported in C, the named constants
+@code{C_FLOAT128} and @code{C_FLOAT128_COMPLEX} are defined.
 
 @multitable @columnfractions .15 .35 .35 .35
 @headitem Fortran Type  @tab Named constant         @tab C type                                @tab Extension
@@ -15225,11 +15225,11 @@ Furthermore, if @code{__float128} is supported in C, the named constants
 @item @code{REAL}   @tab @code{C_FLOAT}         @tab @code{float}
 @item @code{REAL}   @tab @code{C_DOUBLE}        @tab @code{double}
 @item @code{REAL}   @tab @code{C_LONG_DOUBLE}   @tab @code{long double}
-@item @code{REAL}   @tab @code{C_FLOAT128}      @tab @code{__float128}                    @tab Ext.
+@item @code{REAL}   @tab @code{C_FLOAT128}      @tab @code{_Float128}                    @tab Ext.
 @item @code{COMPLEX}@tab @code{C_FLOAT_COMPLEX} @tab @code{float _Complex}
 @item @code{COMPLEX}@tab @code{C_DOUBLE_COMPLEX}@tab @code{double _Complex}
 @item @code{COMPLEX}@tab @code{C_LONG_DOUBLE_COMPLEX}@tab @code{long double _Complex}
-@item @code{REAL}   @tab @code{C_FLOAT128_COMPLEX}   @tab @code{__float128 _Complex}      @tab Ext.
+@item @code{COMPLEX}@tab @code{C_FLOAT128_COMPLEX}   @tab @code{_Float128 _Complex}      @tab Ext.
 @item @code{LOGICAL}@tab @code{C_BOOL}          @tab @code{_Bool}
 @item @code{CHARACTER}@tab @code{C_CHAR}        @tab @code{char}
 @end multitable
diff --git a/gcc/fortran/iso-c-binding.def b/gcc/fortran/iso-c-binding.def
index e65c750..50256fe 100644
--- a/gcc/fortran/iso-c-binding.def
+++ b/gcc/fortran/iso-c-binding.def
@@ -116,7 +116,7 @@ NAMED_REALCST (ISOCBINDING_LONG_DOUBLE, "c_long_double", \
                get_real_kind_from_node (long_double_type_node), GFC_STD_F2003)
 
 /* GNU Extension.  Note that the equivalence here is specifically to
-   the IEEE 128-bit type __float128; if that does not map onto a type
+   the IEEE 128-bit type _Float128; if that does not map onto a type
    otherwise supported by the Fortran front end, get_real_kind_from_node
    will reject it as unsupported.  */
 NAMED_REALCST (ISOCBINDING_FLOAT128, "c_float128", \
diff --git a/gcc/fortran/trans-intrinsic.c b/gcc/fortran/trans-intrinsic.c
index 46670ba..42a995b 100644
--- a/gcc/fortran/trans-intrinsic.c
+++ b/gcc/fortran/trans-intrinsic.c
@@ -175,7 +175,7 @@ gfc_builtin_decl_for_float_kind (enum built_in_function double_built_in,
 
   if (gfc_real_kinds[i].c_float128)
     {
-      /* For __float128, the story is a bit different, because we return
+      /* For _Float128, the story is a bit different, because we return
 	 a decl to a library function rather than a built-in.  */
       gfc_intrinsic_map_t *m;
       for (m = gfc_intrinsic_map; m->double_built_in != double_built_in ; m++)
@@ -387,7 +387,7 @@ build_round_expr (tree arg, tree restype)
   resprec = TYPE_PRECISION (restype);
 
   /* Depending on the type of the result, choose the int intrinsic (iround,
-     available only as a builtin, therefore cannot use it for __float128), long
+     available only as a builtin, therefore cannot use it for _Float128), long
      int intrinsic (lround family) or long long intrinsic (llround).  If we
      don't have an appropriate function that converts directly to the integer
      type (such as kind == 16), just use ROUND, and then convert the result to
@@ -689,7 +689,7 @@ gfc_build_intrinsic_lib_fndecls (void)
   if (gfc_real16_is_float128)
   {
     /* If we have soft-float types, we create the decls for their
-       C99-like library functions.  For now, we only handle __float128
+       C99-like library functions.  For now, we only handle _Float128
        q-suffixed functions.  */
 
     tree type, complex_type, func_1, func_2, func_cabs, func_frexp;
diff --git a/gcc/fortran/trans-types.h b/gcc/fortran/trans-types.h
index 3b45ce2..6804bfe 100644
--- a/gcc/fortran/trans-types.h
+++ b/gcc/fortran/trans-types.h
@@ -55,7 +55,7 @@ extern GTY(()) tree gfc_charlen_type_node;
 
 /* The following flags give us information on the correspondence of
    real (and complex) kinds with C floating-point types long double
-   and __float128.  */
+   and _Float128.  */
 extern bool gfc_real16_is_float128;
 
 enum gfc_packed {
diff --git a/gcc/testsuite/gfortran.dg/PR100914.c b/gcc/testsuite/gfortran.dg/PR100914.c
index c6bd973..ea339e7 100644
--- a/gcc/testsuite/gfortran.dg/PR100914.c
+++ b/gcc/testsuite/gfortran.dg/PR100914.c
@@ -5,7 +5,6 @@
 #include <stdbool.h>
 #include <stdio.h>
 #include <math.h>
-#include <quadmath.h>
 
 #include <ISO_Fortran_binding.h>
 
@@ -29,7 +28,7 @@
 #define CMPLXL(x, y) ((long double complex)((long double)(x) + (long double complex)I * (long double)(y)))
 
 #undef CMPLX
-#define CMPLX(x, y) ((__complex128 )((double)(x) + (double complex)I * (double)(y)))
+#define CMPLX(x, y) ((_Float128 _Complex )((double)(x) + (double complex)I * (double)(y)))
 
 #define N 11
 #define M 7
@@ -37,7 +36,7 @@
 typedef float _Complex c_float_complex;
 typedef double _Complex c_double_complex;
 typedef long double _Complex c_long_double_complex;
-typedef __complex128 c_float128_complex;
+typedef _Float128 _Complex c_float128_complex;
 
 bool c_vrfy_c_float_complex (const CFI_cdesc_t *restrict);
 
diff --git a/gcc/testsuite/gfortran.dg/PR100914.f90 b/gcc/testsuite/gfortran.dg/PR100914.f90
index 64b3335..d8057fd 100644
--- a/gcc/testsuite/gfortran.dg/PR100914.f90
+++ b/gcc/testsuite/gfortran.dg/PR100914.f90
@@ -2,6 +2,7 @@
 ! { dg-do run { xfail { { x86_64*-*-* i?86*-*-* } && longdouble128 } } }
 ! { dg-additional-sources PR100914.c }
 ! { dg-require-effective-target fortran_real_c_float128 }
+! { dg-additional-options "-Wno-pedantic" }
 !
 ! Test the fix for PR100914
 ! 
diff --git a/gcc/testsuite/gfortran.dg/c-interop/typecodes-array-float128-c.c b/gcc/testsuite/gfortran.dg/c-interop/typecodes-array-float128-c.c
index d081feb..4fcb6e2 100644
--- a/gcc/testsuite/gfortran.dg/c-interop/typecodes-array-float128-c.c
+++ b/gcc/testsuite/gfortran.dg/c-interop/typecodes-array-float128-c.c
@@ -32,7 +32,7 @@ void
 ctest (CFI_cdesc_t *arg_float128,
        CFI_cdesc_t *arg_complex128)
 {
-  check (arg_float128, sizeof (__float128), CFI_type_float128);
-  check (arg_complex128, sizeof (__float128) * 2,
+  check (arg_float128, sizeof (_Float128), CFI_type_float128);
+  check (arg_complex128, sizeof (_Float128) * 2,
 	 CFI_type_float128_Complex);
 }
diff --git a/gcc/testsuite/gfortran.dg/c-interop/typecodes-sanity-c.c b/gcc/testsuite/gfortran.dg/c-interop/typecodes-sanity-c.c
index a1d044b..90f0b20 100644
--- a/gcc/testsuite/gfortran.dg/c-interop/typecodes-sanity-c.c
+++ b/gcc/testsuite/gfortran.dg/c-interop/typecodes-sanity-c.c
@@ -23,8 +23,7 @@ static struct tc_info tc_table[] =
 {
   /* Extension types.
      Note there is no portable C equivalent type for CFI_type_ucs4_char type
-     (4-byte Unicode characters), and GCC rejects "__float128 _Complex",
-     so this is kind of hacky...  */
+     (4-byte Unicode characters), so this is kind of hacky...  */
 #if CFI_type_int128_t > 0
   { CFI_type_int128_t, "CFI_type_int128_t",
     sizeof (__int128), 1 },
@@ -38,9 +37,9 @@ static struct tc_info tc_table[] =
 #endif
 #if CFI_type_float128 > 0  
   { CFI_type_float128, "CFI_type_float128",
-    sizeof (__float128), 1 },
+    sizeof (_Float128), 1 },
   { CFI_type_float128_Complex, "CFI_type_float128_Complex",
-    sizeof (__float128) * 2, 1 },
+    sizeof (_Float128 _Complex), 1 },
 #endif
 #if CFI_type_cfunptr > 0  
   { CFI_type_cfunptr, "CFI_type_cfunptr",
diff --git a/gcc/testsuite/gfortran.dg/c-interop/typecodes-scalar-float128-c.c b/gcc/testsuite/gfortran.dg/c-interop/typecodes-scalar-float128-c.c
index f1833aa..7eafa93 100644
--- a/gcc/testsuite/gfortran.dg/c-interop/typecodes-scalar-float128-c.c
+++ b/gcc/testsuite/gfortran.dg/c-interop/typecodes-scalar-float128-c.c
@@ -31,8 +31,8 @@ void
 ctest (CFI_cdesc_t *arg_float128,
        CFI_cdesc_t *arg_complex128)
 {
-  check (arg_float128, sizeof (__float128), CFI_type_float128);
-  check (arg_complex128, sizeof (__float128) * 2,
+  check (arg_float128, sizeof (_Float128), CFI_type_float128);
+  check (arg_complex128, sizeof (_Float128) * 2,
 	 CFI_type_float128_Complex);
 }
 
diff --git a/gcc/testsuite/lib/target-supports.exp b/gcc/testsuite/lib/target-supports.exp
index 8697ceb..f11c4e6 100644
--- a/gcc/testsuite/lib/target-supports.exp
+++ b/gcc/testsuite/lib/target-supports.exp
@@ -1578,8 +1578,8 @@ proc check_effective_target_fortran_real_10 { } {
 
 # Return 1 if the target supports Fortran real kind C_FLOAT128,
 # 0 otherwise.  This differs from check_effective_target_fortran_real_16
-# because __float128 has the additional requirement that it be the
-# 128-bit IEEE encoding; even if __float128 is available in C, it may not
+# because _Float128 has the additional requirement that it be the
+# 128-bit IEEE encoding; even if _Float128 is available in C, it may not
 # have a corresponding Fortran kind on targets (PowerPC) that use some 
 # other encoding for long double/TFmode/real(16).
 proc check_effective_target_fortran_real_c_float128 { } {
diff --git a/libgfortran/ISO_Fortran_binding.h b/libgfortran/ISO_Fortran_binding.h
index a3c6f80..276d958 100644
--- a/libgfortran/ISO_Fortran_binding.h
+++ b/libgfortran/ISO_Fortran_binding.h
@@ -281,7 +281,7 @@ extern int CFI_setpointer (CFI_cdesc_t *, CFI_cdesc_t *, const CFI_index_t []);
 #define CFI_type_long_double (CFI_type_Real + (10 << CFI_type_kind_shift))
 #define CFI_type_long_double_Complex (CFI_type_Complex + (10 << CFI_type_kind_shift))
 
-/* This is the IEEE 128-bit encoding, same as float128.  */
+/* This is the IEEE 128-bit encoding, same as _Float128.  */
 #elif (__CFI_LDBL_MANT_DIG__ == 113 \
        && __CFI_LDBL_MIN_EXP__ == -16381 \
        && __CFI_LDBL_MAX_EXP__ == 16384)
@@ -303,7 +303,7 @@ extern int CFI_setpointer (CFI_cdesc_t *, CFI_cdesc_t *, const CFI_index_t []);
 #error "Can't determine kind of long double"
 #endif
 
-/* Similarly for __float128.  This always refers to the IEEE encoding
+/* Similarly for _Float128.  This always refers to the IEEE encoding
    and not some other 128-bit representation, so if we already used
    kind 16 for a non-IEEE representation, this one must be unsupported
    in Fortran even if it's available in C.  */
diff --git a/libgfortran/runtime/ISO_Fortran_binding.c b/libgfortran/runtime/ISO_Fortran_binding.c
index 0e1a419..e01cc65 100644
--- a/libgfortran/runtime/ISO_Fortran_binding.c
+++ b/libgfortran/runtime/ISO_Fortran_binding.c
@@ -222,7 +222,7 @@ gfc_desc_to_cfi_desc (CFI_cdesc_t **d_ptr, const gfc_array_void *s)
      elem_len and not the kind, we get into trouble with long double kinds
      that do not correspond directly to the elem_len, specifically the
      kind 10 80-bit long double on x86 targets.  On x86_64, this has size
-     16 and cannot be differentiated from true __float128.  Prefer the
+     16 and cannot be differentiated from true _Float128.  Prefer the
      standard long double type over the GNU extension in that case.  */
   if (d->type == CFI_type_Real && kind == sizeof (long double))
     d->type = CFI_type_long_double;

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH, Fortran] Use _Float128 rather than __float128 for c_float128 kind
  2021-09-17  4:02         ` [PATCH, Fortran] Use _Float128 rather than __float128 for c_float128 kind Sandra Loosemore
@ 2021-09-17  8:27           ` Tobias Burnus
  0 siblings, 0 replies; 10+ messages in thread
From: Tobias Burnus @ 2021-09-17  8:27 UTC (permalink / raw)
  To: Sandra Loosemore, fortran, gcc-patches

On 17.09.21 06:02, Sandra Loosemore wrote:
> On 9/5/21 11:20 PM, Sandra Loosemore wrote:
>> Unless the aarch64 maintainers think it is a bug that __float128 is
>> not supported, I think the right solution here is [...] to tie the
>> C_FLOAT128 kind to _Float128 rather than __float128, [...]
>
> Here's a new patch that does this.  I've tested it on
> x86_64-linux-gnu, powerpc64le-linux-gnu, and aarch64-linux-gnu, and it
> does fix the previously reported failure compiling
> gfortran.dg/PR100914.c on aarch64.  OK to commit?

LGTM – thanks!

Tobias

-----------------
Siemens Electronic Design Automation GmbH; Anschrift: Arnulfstraße 201, 80634 München; Gesellschaft mit beschränkter Haftung; Geschäftsführer: Thomas Heurung, Frank Thürauf; Sitz der Gesellschaft: München; Registergericht München, HRB 106955

^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2021-09-17  8:27 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-09-05  2:30 [PATCH, Fortran] Skip gfortran.dg/PR100914.f90 on targets that don't provide quadmath.h Sandra Loosemore
2021-09-05 13:31 ` H.J. Lu
2021-09-05 18:02   ` Sandra Loosemore
2021-09-06  1:29     ` H.J. Lu
2021-09-06  5:20       ` Sandra Loosemore
2021-09-06  7:06         ` Christophe Lyon
2021-09-06 10:18         ` Tobias Burnus
2021-09-06 16:31         ` Joseph Myers
2021-09-17  4:02         ` [PATCH, Fortran] Use _Float128 rather than __float128 for c_float128 kind Sandra Loosemore
2021-09-17  8:27           ` Tobias Burnus

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).