[Patch] Testsuite/Fortran: gfortran.dg/pr96711.f90 - fix expected value for PowerPC [PR96983]

[Patch] Testsuite/Fortran: gfortran.dg/pr96711.f90 - fix expected value for PowerPC [PR96983]

[Tobias Burnus]

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

Regarding gfortran.dg/pr96711.f90:

On my x86-64-gnu-linux, it PASSes.
On our powerpc64le-linux-gnu it FAILS with
'STOP 3' (→ also scan-dump count) and 'STOP 4'.

Contrary to PR96983's bug summary, I don't get an ICE.


On powerpc64le-linux-gnu, the following condition evaluates true (→ 'STOP 3'):

    real(16)               :: y   ! 128bit REAL
    integer(16), parameter :: k2 = nint (2 / epsilon (y), kind(k2))
    integer(16), parameter :: m2 = 10384593717069655257060992658440192_16 !2**113
    if (k2 /= m2) stop 3

On x86_64-linux-gnu, k2 == m2 — but on powerpc64le-linux-gnu,
k2 == 2**106 instead of 2**113.

My solution is to permit also 2**106 besides 2**113.

@PowerPC maintainers: Does this make sense? – It seems to work on our PowerPC
but with all the new 'long double' changes, does it also work for you?

@All, Harald: Does the attached patch make sense?

Tobias

PS: I find the PR a bit confusing – there are some GCC 11 commits
and a GCC 12 commit but no real explanation what now works and what
still fails.

-----------------
Mentor Graphics (Deutschland) GmbH, Arnulfstrasse 201, 80634 München Registergericht München HRB 106955, Geschäftsführer: Thomas Heurung, Frank Thürauf

[-- Attachment #2: fix-pr96711.diff --]
[-- Type: text/x-patch, Size: 1645 bytes --]

Testsuite/Fortran: gfortran.dg/pr96711.f90 - fix expected value for PowerPC [PR96983]

gcc/testsuite/ChangeLog:

	PR fortran/96983
	* gfortran.dg/pr96711.f90:

diff --git a/gcc/testsuite/gfortran.dg/pr96711.f90 b/gcc/testsuite/gfortran.dg/pr96711.f90
index 3761a8ea416..0597ff2e37e 100644
--- a/gcc/testsuite/gfortran.dg/pr96711.f90
+++ b/gcc/testsuite/gfortran.dg/pr96711.f90
@@ -1,28 +1,29 @@
 ! { dg-do run }
 ! { dg-require-effective-target fortran_integer_16 }
 ! { dg-require-effective-target fortran_real_16 }
 ! { dg-additional-options "-fdump-tree-original" }
 ! { dg-final { scan-tree-dump-times "_gfortran_stop_numeric" 2 "original" } }
 !
 ! PR fortran/96711 - ICE on NINT() Function
 
 program p
   implicit none
   real(8)                :: x
   real(16)               :: y
   integer(16), parameter :: k1 = nint (2 / epsilon (x), kind(k1))
   integer(16), parameter :: k2 = nint (2 / epsilon (y), kind(k2))
   integer(16), parameter :: m1 = 9007199254740992_16                    !2**53
-  integer(16), parameter :: m2 = 10384593717069655257060992658440192_16 !2**113
+  integer(16), parameter :: m2 = 10384593717069655257060992658440192_16 !2**113  ! Some systems like x86-64
+  integer(16), parameter :: m2a = 81129638414606681695789005144064_16   !2**106  ! Some systems like PowerPC
   integer(16), volatile  :: m
   x = 2 / epsilon (x)
   y = 2 / epsilon (y)
   m = nint (x, kind(m))
 ! print *, m
   if (k1 /= m1) stop 1
   if (m  /= m1) stop 2
   m = nint (y, kind(m))
 ! print *, m
-  if (k2 /= m2) stop 3
-  if (m  /= m2) stop 4
+  if (k2 /= m2 .and. k2 /= m2a) stop 3
+  if (m  /= m2 .and. m /= m2a) stop 4
 end program

Re: [Patch] Testsuite/Fortran: gfortran.dg/pr96711.f90 - fix expected value for PowerPC [PR96983]

[Segher Boessenkool]

On Wed, May 19, 2021 at 02:32:02PM +0200, Tobias Burnus wrote:
> Regarding gfortran.dg/pr96711.f90:
> 
> On my x86-64-gnu-linux, it PASSes.
> On our powerpc64le-linux-gnu it FAILS with
> 'STOP 3' (→ also scan-dump count) and 'STOP 4'.
> 
> Contrary to PR96983's bug summary, I don't get an ICE.
> 
> 
> On powerpc64le-linux-gnu, the following condition evaluates true (→ 'STOP 
> 3'):
> 
>    real(16)               :: y   ! 128bit REAL
>    integer(16), parameter :: k2 = nint (2 / epsilon (y), kind(k2))
>    integer(16), parameter :: m2 = 10384593717069655257060992658440192_16 
>    !2**113
>    if (k2 /= m2) stop 3
> 
> On x86_64-linux-gnu, k2 == m2 — but on powerpc64le-linux-gnu,
> k2 == 2**106 instead of 2**113.
> 
> My solution is to permit also 2**106 besides 2**113.
> 
> @PowerPC maintainers: Does this make sense? – It seems to work on our 
> PowerPC
> but with all the new 'long double' changes, does it also work for you?

I do not understand Fortran well enough, could you explain what the code
is supposed to do?

> 	PR fortran/96983
> 	* gfortran.dg/pr96711.f90:

You're missing the actual entry here, fwiw.

> -  integer(16), parameter :: m2 = 10384593717069655257060992658440192_16 !2**113
> +  integer(16), parameter :: m2 = 10384593717069655257060992658440192_16 !2**113  ! Some systems like x86-64
> +  integer(16), parameter :: m2a = 81129638414606681695789005144064_16   !2**106  ! Some systems like PowerPC

If you use double-double ("ibm long double") a number is represented as
the sum of two double precision numbers, while if you use IEEE quad
precision floating point you get a 112-bit fraction (and a leading one).
The most significant of the two DP numbers is the whole rounded to DP.
The actual precision varies, it depends on various factors :-/


Segher


>    integer(16), volatile  :: m
>    x = 2 / epsilon (x)
>    y = 2 / epsilon (y)
>    m = nint (x, kind(m))
>  ! print *, m
>    if (k1 /= m1) stop 1
>    if (m  /= m1) stop 2
>    m = nint (y, kind(m))
>  ! print *, m
> -  if (k2 /= m2) stop 3
> -  if (m  /= m2) stop 4
> +  if (k2 /= m2 .and. k2 /= m2a) stop 3
> +  if (m  /= m2 .and. m /= m2a) stop 4
>  end program

Re: [Patch] Testsuite/Fortran: gfortran.dg/pr96711.f90 - fix expected value for PowerPC [PR96983]

[Tobias Burnus]

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

Hi Segher,

Quick version: Jump to the new patch, which I like much more.
Longer version:
On 19.05.21 17:15, Segher Boessenkool wrote:
>>     real(16)               :: y   ! 128bit REAL
>>     integer(16), parameter :: k2 = nint (2 / epsilon (y), kind(k2))
>>     integer(16), parameter :: m2 = 10384593717069655257060992658440192_16
>>     !2**113
>>     if (k2 /= m2) stop 3
>>
>> On x86_64-linux-gnu, k2 == m2 — but on powerpc64le-linux-gnu,
>> k2 == 2**106 instead of 2**113.
>>
>> My solution is to permit also 2**106 besides 2**113.
> I do not understand Fortran well enough, could you explain what the code
> is supposed to do?

First, 2_16 means the integer '2' of the integer kind '16', i.e. int128_t type.

The original bug report (PR96983) was that 'nint'
with the 16byte floating point/quad-precision was giving an ICE
and the complaint was that there was no testcase for the value.

And I think this testcase tries to ensure that the result of 'nint'
both at compile time and at runtime matches what should be the result.

(Quotes from the Fortran standard, augumenty by what the source code does.)

'nint' does "__builtin_round" is available: "The result is the integer
nearest A, or if there are two integers equally near A, the result
is whichever such integer has the greater magnitude" – and in this
testcase, the argument is a quad-precision/16byte/128bit floating-point
number and the result is a 128bit integer.

('a**b' is the Fortran syntax for: 'a' raised to the power of 'b'.)

This testcase does:

nint(2/epsilon(y)). Here, 'epsilon' is the
"Model number that is small compared to 1."
Namely: b**(p-1) = '(radix)**(1-digits)'
alias 'real_format *fmt = REAL_MODE_FORMAT (mode)'
with radix = fmt->b  and digits = fmt->p;

[b**(p-1) is from the Fortran standard but 'b' and 'p' also match the
ME/target names, while radix/digits matches the FE names and also the
Fortran intrinsic inquiry function names.]

This is for radix = 2 equivalent to:

2/2**(1-digits) = 2*2**(digits-1) = 2**(digits)

On x86-64, digits == fmt->p == 113.

Our powerpc64le gives digits == 106.

  * * *

Having written all this, I wonder why we don't just
rely on the assumption that '2**digit(x)' works – and use this
to generate the valid.

Namely, as the attached updated patch does.

As I like that patch and believe it is obvious, I intent to
commit it as such – unless there are further comments.

It passes on both x86-64-gnu-linux and powerpc64le-none-linux-gnu.
I think the radix == 2 is a good bet, but if we ever run into issues,
it can also be changed to use radix(...) as well ...

Tobias

-----------------
Mentor Graphics (Deutschland) GmbH, Arnulfstrasse 201, 80634 München Registergericht München HRB 106955, Geschäftsführer: Thomas Heurung, Frank Thürauf

[-- Attachment #2: fix-pr96711-v2.diff --]
[-- Type: text/x-patch, Size: 1722 bytes --]

Testsuite/Fortran: gfortran.dg/pr96711.f90 - fix expected value for PowerPC [PR96983]

gcc/testsuite/ChangeLog:

	PR fortran/96983
	* gfortran.dg/pr96711.f90: Use 2**digit(x) instead of a hard-coded value;
	add comments regarding what the code does.

diff --git a/gcc/testsuite/gfortran.dg/pr96711.f90 b/gcc/testsuite/gfortran.dg/pr96711.f90
index 3761a8ea416..ab7707d0120 100644
--- a/gcc/testsuite/gfortran.dg/pr96711.f90
+++ b/gcc/testsuite/gfortran.dg/pr96711.f90
@@ -1,28 +1,30 @@
 ! { dg-do run }
 ! { dg-require-effective-target fortran_integer_16 }
 ! { dg-require-effective-target fortran_real_16 }
 ! { dg-additional-options "-fdump-tree-original" }
 ! { dg-final { scan-tree-dump-times "_gfortran_stop_numeric" 2 "original" } }
 !
 ! PR fortran/96711 - ICE on NINT() Function
 
 program p
   implicit none
   real(8)                :: x
   real(16)               :: y
+  ! Assume radix(x) == 2
+  ! 2/epsilon(x) = 2/(radix(x)**(1-digits(x)) = 2**digits(x) with that assumption
   integer(16), parameter :: k1 = nint (2 / epsilon (x), kind(k1))
   integer(16), parameter :: k2 = nint (2 / epsilon (y), kind(k2))
-  integer(16), parameter :: m1 = 9007199254740992_16                    !2**53
-  integer(16), parameter :: m2 = 10384593717069655257060992658440192_16 !2**113
+  integer(16), parameter :: m1 = 2_16**digits(x) ! Usually 2**53
+  integer(16), parameter :: m2 = 2_16**digits(y) ! Might be 2**113 or 2**106 or ... depending on the system
   integer(16), volatile  :: m
   x = 2 / epsilon (x)
   y = 2 / epsilon (y)
   m = nint (x, kind(m))
 ! print *, m
   if (k1 /= m1) stop 1
   if (m  /= m1) stop 2
   m = nint (y, kind(m))
 ! print *, m
   if (k2 /= m2) stop 3
   if (m  /= m2) stop 4
 end program

Re: [Patch] Testsuite/Fortran: gfortran.dg/pr96711.f90 - fix expected value for PowerPC [PR96983]

[Bernhard Reutner-Fischer]

On Wed, 19 May 2021 20:35:26 +0200
Tobias Burnus <tobias@codesourcery.com> wrote:

> Hi Segher,
> 
> Quick version: Jump to the new patch, which I like much more.

> Namely, as the attached updated patch does.
> 
> As I like that patch and believe it is obvious, I intent to

/intent/s/tent/tend/

> commit it as such – unless there are further comments.

No real comment except that it sounds odd to arrive at 53 instead of
the quad bits precision on an arch that allegedly does hardware FP?
Did not look close though so i claim to haven't said a word, let alone
2*2 nor four.

thanks,
> 
> It passes on both x86-64-gnu-linux and powerpc64le-none-linux-gnu.
> I think the radix == 2 is a good bet, but if we ever run into issues,
> it can also be changed to use radix(...) as well ...
> 
> Tobias
> 
> -----------------
> Mentor Graphics (Deutschland) GmbH, Arnulfstrasse 201, 80634 München Registergericht München HRB 106955, Geschäftsführer: Thomas Heurung, Frank Thürauf

Re: [Patch] Testsuite/Fortran: gfortran.dg/pr96711.f90 - fix expected value for PowerPC [PR96983]

[Bernhard Reutner-Fischer]

On Wed, 19 May 2021 22:39:13 +0200
Bernhard Reutner-Fischer <rep.dot.nop@gmail.com> wrote:

> On Wed, 19 May 2021 20:35:26 +0200
> Tobias Burnus <tobias@codesourcery.com> wrote:

> > commit it as such – unless there are further comments.  
> 
> No real comment except ..

why don't we end up with IEEE binary128 quadruple precision here per
default. Suspicious, no?
thanks,

Re: [Patch] Testsuite/Fortran: gfortran.dg/pr96711.f90 - fix expected value for PowerPC [PR96983]

[Tobias Burnus]

On 19.05.21 22:39, Bernhard Reutner-Fischer wrote:
> On Wed, 19 May 2021 20:35:26 +0200
> Tobias Burnus <tobias@codesourcery.com> wrote:
>> As I like that patch and believe it is obvious, I intent to
> /intent/s/tent/tend/
?
> No real comment except that it sounds odd to arrive at 53 instead of
> the quad bits precision on an arch that allegedly does hardware FP?
   real(8)                :: x
   integer(16), parameter :: m1 = 2_16**digits(x) ! Usually 2**53

i.e. 'x' is double precision not quad precision.

Tobias
-----------------
Mentor Graphics (Deutschland) GmbH, Arnulfstrasse 201, 80634 München Registergericht München HRB 106955, Geschäftsführer: Thomas Heurung, Frank Thürauf

Re: [Patch] Testsuite/Fortran: gfortran.dg/pr96711.f90 - fix expected value for PowerPC [PR96983]

[Segher Boessenkool]

Hi!

On Wed, May 19, 2021 at 08:35:26PM +0200, Tobias Burnus wrote:
> On 19.05.21 17:15, Segher Boessenkool wrote:
> >>    real(16)               :: y   ! 128bit REAL
> >>    integer(16), parameter :: k2 = nint (2 / epsilon (y), kind(k2))
> >>    integer(16), parameter :: m2 = 10384593717069655257060992658440192_16
> >>    !2**113
> >>    if (k2 /= m2) stop 3
> >>
> >>On x86_64-linux-gnu, k2 == m2 — but on powerpc64le-linux-gnu,
> >>k2 == 2**106 instead of 2**113.
> >>
> >>My solution is to permit also 2**106 besides 2**113.
> >I do not understand Fortran well enough, could you explain what the code
> >is supposed to do?
> 
> First, 2_16 means the integer '2' of the integer kind '16', i.e. int128_t 
> type.

Ah ok.

> And I think this testcase tries to ensure that the result of 'nint'
> both at compile time and at runtime matches what should be the result.

That is the main thing I missed :-)

> ('a**b' is the Fortran syntax for: 'a' raised to the power of 'b'.)

I know, I use it all the time :-)  Many other languages have this
nowadays btw.  It is such a succinct notation :-)

> nint(2/epsilon(y)). Here, 'epsilon' is the
> "Model number that is small compared to 1."
> Namely: b**(p-1) = '(radix)**(1-digits)'
> alias 'real_format *fmt = REAL_MODE_FORMAT (mode)'
> with radix = fmt->b  and digits = fmt->p;
> 
> [b**(p-1) is from the Fortran standard but 'b' and 'p' also match the
> ME/target names, while radix/digits matches the FE names and also the
> Fortran intrinsic inquiry function names.]
> 
> This is for radix = 2 equivalent to:
> 
> 2/2**(1-digits) = 2*2**(digits-1) = 2**(digits)
> 
> On x86-64, digits == fmt->p == 113.

And the same on powerpc64le with -mabi=ieeelongdouble, presumably.

> Our powerpc64le gives digits == 106.

It stil defaults to -mabi=ibmlongdouble.

> Having written all this, I wonder why we don't just
> rely on the assumption that '2**digit(x)' works – and use this
> to generate the valid.

If that works (and I have no reason to believe it won't), that is as
simple a solution as you will find :-)

> As I like that patch and believe it is obvious, I intent to
> commit it as such – unless there are further comments.

> It passes on both x86-64-gnu-linux and powerpc64le-none-linux-gnu.
> I think the radix == 2 is a good bet, but if we ever run into issues,
> it can also be changed to use radix(...) as well ...

I don't think any GCC target does 10 or 16 by default, and less chance
in Fortran even :-)

Thanks!


Segher

Aw: [Patch] Testsuite/Fortran: gfortran.dg/pr96711.f90 - fix expected value for PowerPC [PR96983]

[Harald Anlauf]

Hi Tobias,

> @All, Harald: Does the attached patch make sense?

first of all: sorry for the really badly designed testcase.
The best solution has already been discussed in this thread,
which is to replace

  integer(16), parameter :: m1 = 9007199254740992_16                    !2**53
  integer(16), parameter :: m2 = 10384593717069655257060992658440192_16 !2**113

by

  integer(16), parameter :: m1 = 2_16 ** digits (x) ! IEEE: 2**53
  integer(16), parameter :: m2 = 2_16 ** digits (y) ! IEEE: 2**113

The motivation was to test that compile-time and run-time produce the
same correct result, as well as verifying that the user gets what he/she
would naively expect from the intrinsic.  There should be no hidden
double conversion that might e.g. truncate.

I decided for the largest real values which are exactly representable
also as integer, and for which the rounding operation should always
reproduce the expected result.

E.g.  nint (x) - nint (x-1) should give 1, while nint (x) - nint (x+1)
might give 0, which happens for the chosen values on x86.

I had this in my mind, but decided to drop this because I had no idea
if there are exotic, non-IEEE, or other implementations which would
fail on this.

Thanks for fixing this!

Harald