public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
* [PATCH] fortran/69121 -- Make IEEE_SCALB generic
@ 2018-12-20 21:53 Steve Kargl
  2018-12-21  6:46 ` Steve Kargl
  0 siblings, 1 reply; 13+ messages in thread
From: Steve Kargl @ 2018-12-20 21:53 UTC (permalink / raw)
  To: fortran, gcc-patches

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

The attached patch has been tested on x86_64-*-freebsd.

It provides the missing pieces for IEEE_SCALB to allow
it to be generic function.  Briefly, when FX contributed
IEEE_SCALB, he implemented a casting of the integer
argument to the default integer kind.  For integer(1)
and integer(2), this is a simple cast.  For integer(8)
and integer(16), he first checks to see if the value 
is in the range of default integer kind.  So, one
essentially has

ieee_scalb(x, 2_1) --> ieee_scalb(x, int(2_1, 4)) 
ieee_scalb(x, 2_8) --> ieee_scalb(x, int(max(min(2_8,huge(0)),-huge(0)), 4)

Unfortnately, the interface exposed by the IEEE_ARITHMETIC
module did not allow for non-default integer kind arguments.
The patch corrects that omission.

OK to commit?

2018-12-20  Steven G. Kargl  <kargl@gcc.gnu.org>

	PR fortran/69121
	* libgfortran/ieee/ieee_arithmetic.F90: Provide missing functions
	in interface for IEEE_SCALB.

2018-12-20  Steven G. Kargl  <kargl@gcc.gnu.org>

	PR fortran/69121
	* gfortran.dg/ieee/ieee_9.f90: New test.
-- 
Steve

[-- Attachment #2: ieee.diff --]
[-- Type: text/x-diff, Size: 7957 bytes --]

Index: libgfortran/ieee/ieee_arithmetic.F90
===================================================================
--- libgfortran/ieee/ieee_arithmetic.F90	(revision 267312)
+++ libgfortran/ieee/ieee_arithmetic.F90	(working copy)
@@ -532,37 +532,170 @@ REM_MACRO(4,4,4)
   ! IEEE_SCALB
 
   interface
-    elemental real(kind=4) function _gfortran_ieee_scalb_4 (X, I)
+#ifdef HAVE_GFC_INTEGER_16
+#ifdef HAVE_GFC_REAL_16
+    elemental real(kind=16) function _gfortran_ieee_scalb_16_16 (X, I)
+      real(kind=16), intent(in) :: X
+      integer(kind=16), intent(in) :: I
+    end function
+#endif
+#ifdef HAVE_GFC_REAL_10
+    elemental real(kind=10) function _gfortran_ieee_scalb_10_16 (X, I)
+      real(kind=10), intent(in) :: X
+      integer(kind=16), intent(in) :: I
+    end function
+#endif
+    elemental real(kind=8) function _gfortran_ieee_scalb_8_16 (X, I)
+      real(kind=8), intent(in) :: X
+      integer(kind=16), intent(in) :: I
+    end function
+    elemental real(kind=4) function _gfortran_ieee_scalb_4_16 (X, I)
       real(kind=4), intent(in) :: X
-      integer, intent(in) :: I
+      integer(kind=16), intent(in) :: I
     end function
-    elemental real(kind=8) function _gfortran_ieee_scalb_8 (X, I)
+#endif
+
+#ifdef HAVE_GFC_INTEGER_8
+#ifdef HAVE_GFC_REAL_16
+    elemental real(kind=16) function _gfortran_ieee_scalb_16_8 (X, I)
+      real(kind=16), intent(in) :: X
+      integer(kind=8), intent(in) :: I
+    end function
+#endif
+#ifdef HAVE_GFC_REAL_10
+    elemental real(kind=10) function _gfortran_ieee_scalb_10_8 (X, I)
+      real(kind=10), intent(in) :: X
+      integer(kind=8), intent(in) :: I
+    end function
+#endif
+    elemental real(kind=8) function _gfortran_ieee_scalb_8_8 (X, I)
       real(kind=8), intent(in) :: X
-      integer, intent(in) :: I
+      integer(kind=8), intent(in) :: I
     end function
+    elemental real(kind=4) function _gfortran_ieee_scalb_4_8 (X, I)
+      real(kind=4), intent(in) :: X
+      integer(kind=8), intent(in) :: I
+    end function
+#endif
+
+#ifdef HAVE_GFC_INTEGER_2
+#ifdef HAVE_GFC_REAL_16
+    elemental real(kind=16) function _gfortran_ieee_scalb_16_2 (X, I)
+      real(kind=16), intent(in) :: X
+      integer(kind=2), intent(in) :: I
+    end function
+#endif
 #ifdef HAVE_GFC_REAL_10
-    elemental real(kind=10) function _gfortran_ieee_scalb_10 (X, I)
+    elemental real(kind=10) function _gfortran_ieee_scalb_10_2 (X, I)
       real(kind=10), intent(in) :: X
-      integer, intent(in) :: I
+      integer(kind=2), intent(in) :: I
     end function
 #endif
+    elemental real(kind=8) function _gfortran_ieee_scalb_8_2 (X, I)
+      real(kind=8), intent(in) :: X
+      integer(kind=2), intent(in) :: I
+    end function
+    elemental real(kind=4) function _gfortran_ieee_scalb_4_2 (X, I)
+      real(kind=4), intent(in) :: X
+      integer(kind=2), intent(in) :: I
+    end function
+#endif
+
+#ifdef HAVE_GFC_INTEGER_1
 #ifdef HAVE_GFC_REAL_16
-    elemental real(kind=16) function _gfortran_ieee_scalb_16 (X, I)
+    elemental real(kind=16) function _gfortran_ieee_scalb_16_1 (X, I)
       real(kind=16), intent(in) :: X
+      integer(kind=1), intent(in) :: I
+    end function
+#endif
+#ifdef HAVE_GFC_REAL_10
+    elemental real(kind=10) function _gfortran_ieee_scalb_10_1 (X, I)
+      real(kind=10), intent(in) :: X
+      integer(kind=1), intent(in) :: I
+    end function
+#endif
+    elemental real(kind=8) function _gfortran_ieee_scalb_8_1 (X, I)
+      real(kind=8), intent(in) :: X
+      integer(kind=1), intent(in) :: I
+    end function
+    elemental real(kind=4) function _gfortran_ieee_scalb_4_1 (X, I)
+      real(kind=4), intent(in) :: X
+      integer(kind=1), intent(in) :: I
+    end function
+#endif
+
+#ifdef HAVE_GFC_REAL_16
+    elemental real(kind=16) function _gfortran_ieee_scalb_16_4 (X, I)
+      real(kind=16), intent(in) :: X
       integer, intent(in) :: I
     end function
 #endif
+#ifdef HAVE_GFC_REAL_10
+    elemental real(kind=10) function _gfortran_ieee_scalb_10_4 (X, I)
+      real(kind=10), intent(in) :: X
+      integer, intent(in) :: I
+    end function
+#endif
+    elemental real(kind=8) function _gfortran_ieee_scalb_8_4 (X, I)
+      real(kind=8), intent(in) :: X
+      integer, intent(in) :: I
+    end function
+    elemental real(kind=4) function _gfortran_ieee_scalb_4_4 (X, I)
+      real(kind=4), intent(in) :: X
+      integer, intent(in) :: I
+    end function
   end interface
 
   interface IEEE_SCALB
     procedure &
+#ifdef HAVE_GFC_INTEGER_16
 #ifdef HAVE_GFC_REAL_16
-      _gfortran_ieee_scalb_16, &
+    _gfortran_ieee_scalb_16_16, &
 #endif
 #ifdef HAVE_GFC_REAL_10
-      _gfortran_ieee_scalb_10, &
+    _gfortran_ieee_scalb_10_16, &
 #endif
-      _gfortran_ieee_scalb_8, _gfortran_ieee_scalb_4
+    _gfortran_ieee_scalb_8_16, &
+    _gfortran_ieee_scalb_4_16, &
+#endif
+#ifdef HAVE_GFC_INTEGER_8
+#ifdef HAVE_GFC_REAL_16
+    _gfortran_ieee_scalb_16_8, &
+#endif
+#ifdef HAVE_GFC_REAL_10
+    _gfortran_ieee_scalb_10_8, &
+#endif
+    _gfortran_ieee_scalb_8_8, &
+    _gfortran_ieee_scalb_4_8, &
+#endif
+#ifdef HAVE_GFC_INTEGER_2
+#ifdef HAVE_GFC_REAL_16
+    _gfortran_ieee_scalb_16_2, &
+#endif
+#ifdef HAVE_GFC_REAL_10
+    _gfortran_ieee_scalb_10_2, &
+#endif
+    _gfortran_ieee_scalb_8_2, &
+    _gfortran_ieee_scalb_4_2, &
+#endif
+#ifdef HAVE_GFC_INTEGER_1
+#ifdef HAVE_GFC_REAL_16
+    _gfortran_ieee_scalb_16_1, &
+#endif
+#ifdef HAVE_GFC_REAL_10
+    _gfortran_ieee_scalb_10_1, &
+#endif
+    _gfortran_ieee_scalb_8_1, &
+    _gfortran_ieee_scalb_4_1, &
+#endif
+#ifdef HAVE_GFC_REAL_16
+    _gfortran_ieee_scalb_16_4, &
+#endif
+#ifdef HAVE_GFC_REAL_10
+    _gfortran_ieee_scalb_10_4, &
+#endif
+      _gfortran_ieee_scalb_8_4, &
+      _gfortran_ieee_scalb_4_4
   end interface
   public :: IEEE_SCALB
 
Index: gcc/testsuite/gfortran.dg/ieee/ieee_9.f90
===================================================================
--- gcc/testsuite/gfortran.dg/ieee/ieee_9.f90	(nonexistent)
+++ gcc/testsuite/gfortran.dg/ieee/ieee_9.f90	(working copy)
@@ -0,0 +1,70 @@
+! { dg-do run }
+program foo
+   use ieee_arithmetic
+   use iso_fortran_env
+   integer i, p
+   real x
+   x = 4
+   i = 4
+
+   if (int8 > 0) then
+      if (real32 > 0) then
+         p = int(ieee_scalb(real(x, real32), int(i, int8)))
+         if (p /= 64) stop 1
+      endif
+      if (real64 > 0) then
+         p = int(ieee_scalb(real(x, real64), int(i, int8)))
+         if (p /= 64) stop 2
+      endif
+      if (real128 > 0) then
+         p = int(ieee_scalb(real(x, real128), int(i, int8)))
+         if (p /= 64) stop 3
+      end if
+   end if
+
+   if (int16 > 0) then
+      if (real32 > 0) then
+         p = int(ieee_scalb(real(x, real32), int(i, int16)))
+         if (p /= 64) stop 4
+      endif
+      if (real64 > 0) then
+         p = int(ieee_scalb(real(x, real64), int(i, int16)))
+         if (p /= 64) stop 5
+      endif
+      if (real128 > 0) then
+         p = int(ieee_scalb(real(x, real128), int(i, int16)))
+         if (p /= 64) stop 6
+      end if
+   end if
+
+   if (int32 > 0) then
+      if (real32 > 0) then
+         p = int(ieee_scalb(real(x, real32), int(i, int32)))
+         if (p /= 64) stop 7
+      endif
+      if (real64 > 0) then
+         p = int(ieee_scalb(real(x, real64), int(i, int32)))
+         if (p /= 64) stop 8
+      endif
+      if (real128 > 0) then
+         p = int(ieee_scalb(real(x, real128), int(i, int32)))
+         if (p /= 64) stop 9
+      end if
+   end if
+
+   if (int64 > 0) then
+      if (real32 > 0) then
+         p = int(ieee_scalb(real(x, real32), int(i, int64)))
+         if (p /= 64) stop 10
+      endif
+      if (real64 > 0) then
+         p = int(ieee_scalb(real(x, real64), int(i, int64)))
+         if (p /= 64) stop 11
+      endif
+      if (real128 > 0) then
+         p = int(ieee_scalb(real(x, real128), int(i, int64)))
+         if (p /= 64) stop 12
+      end if
+   end if
+
+end program foo

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

* Re: [PATCH] fortran/69121 -- Make IEEE_SCALB generic
  2018-12-20 21:53 [PATCH] fortran/69121 -- Make IEEE_SCALB generic Steve Kargl
@ 2018-12-21  6:46 ` Steve Kargl
  2018-12-21  9:08   ` Janne Blomqvist
  0 siblings, 1 reply; 13+ messages in thread
From: Steve Kargl @ 2018-12-21  6:46 UTC (permalink / raw)
  To: fortran, gcc-patches

On Thu, Dec 20, 2018 at 01:47:39PM -0800, Steve Kargl wrote:
> The attached patch has been tested on x86_64-*-freebsd.
> 
> OK to commit?
> 
> 2018-12-20  Steven G. Kargl  <kargl@gcc.gnu.org>
> 
> 	PR fortran/69121
> 	* libgfortran/ieee/ieee_arithmetic.F90: Provide missing functions
> 	in interface for IEEE_SCALB.
> 
> 2018-12-20  Steven G. Kargl  <kargl@gcc.gnu.org>
> 
> 	PR fortran/69121
> 	* gfortran.dg/ieee/ieee_9.f90: New test.

Now, tested on i586-*-freebsd.

-- 
Steve

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

* Re: [PATCH] fortran/69121 -- Make IEEE_SCALB generic
  2018-12-21  6:46 ` Steve Kargl
@ 2018-12-21  9:08   ` Janne Blomqvist
  2018-12-21 15:55     ` Steve Kargl
  2018-12-21 17:59     ` Steve Kargl
  0 siblings, 2 replies; 13+ messages in thread
From: Janne Blomqvist @ 2018-12-21  9:08 UTC (permalink / raw)
  To: Steve Kargl; +Cc: Fortran List, GCC Patches

On Fri, Dec 21, 2018 at 8:22 AM Steve Kargl <
sgk@troutmask.apl.washington.edu> wrote:

> On Thu, Dec 20, 2018 at 01:47:39PM -0800, Steve Kargl wrote:
> > The attached patch has been tested on x86_64-*-freebsd.
> >
> > OK to commit?
> >
> > 2018-12-20  Steven G. Kargl  <kargl@gcc.gnu.org>
> >
> >       PR fortran/69121
> >       * libgfortran/ieee/ieee_arithmetic.F90: Provide missing functions
> >       in interface for IEEE_SCALB.
> >
> > 2018-12-20  Steven G. Kargl  <kargl@gcc.gnu.org>
> >
> >       PR fortran/69121
> >       * gfortran.dg/ieee/ieee_9.f90: New test.
>
> Now, tested on i586-*-freebsd.
>
> --
> Steve
>

Hi, looks ok for trunk.

A few questions popped into my mind while looking into this:

1) Why are none of the _gfortran_ieee_scalb_X_Y functions mentioned in
gfortran.map? I guess they should all be there?

2) Currently all the intrinsics map to the scalbn{,f,l} builtins. However,
when the integer argument is of kind int64 or int128 we should instead use
scalbln{,f,l}. This also applies to other intrinsics that use scalbn under
the hood.

To clarify, fixing these is not a prerequisite for accepting the patch (I
already accepted it), but more like topics for further work.

-- 
Janne Blomqvist

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

* Re: [PATCH] fortran/69121 -- Make IEEE_SCALB generic
  2018-12-21  9:08   ` Janne Blomqvist
@ 2018-12-21 15:55     ` Steve Kargl
  2018-12-21 17:35       ` Thomas Koenig
  2018-12-21 17:59     ` Steve Kargl
  1 sibling, 1 reply; 13+ messages in thread
From: Steve Kargl @ 2018-12-21 15:55 UTC (permalink / raw)
  To: Janne Blomqvist; +Cc: Fortran List, GCC Patches

On Fri, Dec 21, 2018 at 11:07:08AM +0200, Janne Blomqvist wrote:
> On Fri, Dec 21, 2018 at 8:22 AM Steve Kargl <
> sgk@troutmask.apl.washington.edu> wrote:
> 
> > On Thu, Dec 20, 2018 at 01:47:39PM -0800, Steve Kargl wrote:
> > > The attached patch has been tested on x86_64-*-freebsd.
> > >
> > > OK to commit?
> > >
> > > 2018-12-20  Steven G. Kargl  <kargl@gcc.gnu.org>
> > >
> > >       PR fortran/69121
> > >       * libgfortran/ieee/ieee_arithmetic.F90: Provide missing functions
> > >       in interface for IEEE_SCALB.
> > >
> > > 2018-12-20  Steven G. Kargl  <kargl@gcc.gnu.org>
> > >
> > >       PR fortran/69121
> > >       * gfortran.dg/ieee/ieee_9.f90: New test.
> >
> > Now, tested on i586-*-freebsd.
> >
> 
> Hi, looks ok for trunk.
> 
> A few questions popped into my mind while looking into this:
> 
> 1) Why are none of the _gfortran_ieee_scalb_X_Y functions mentioned in
> gfortran.map? I guess they should all be there?
> 
> 2) Currently all the intrinsics map to the scalbn{,f,l} builtins. However,
> when the integer argument is of kind int64 or int128 we should instead use
> scalbln{,f,l}. This also applies to other intrinsics that use scalbn under
> the hood.
> 
> To clarify, fixing these is not a prerequisite for accepting the patch (I
> already accepted it), but more like topics for further work.

Just, sent shorter note in private email to Thomas.

No, I'm adding the missing functions to the INTERFACE.

This will not compile: 

   program foo
   use ieee_arithmetic
   real x
   integer(8) i
   x = 2
   i = 2_8
   print *, ieee_scalb(x,i)
   end program

because the module has a generic interface that does not
include the integer(8) argument.

FX seems to have wanted to avoid the explosion of functions in
the library.  In trans-intrinsic.c (conv_intrinsic_ieee_scalb),
he does a conversion of the integer(8) to an integer(4).
Unfortunately, checking the interface for ieee_scalb occurs
before code generation.  Compiling the above after my patch,
the -ftree-dump-original contains

D.3769 = __builtin_scalbnf (x, (integer(kind=4)) MAX_EXPR <MIN_EXPR <D.3768, 2147483647>, -2147483647>);

-- 
Steve

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

* Re: [PATCH] fortran/69121 -- Make IEEE_SCALB generic
  2018-12-21 15:55     ` Steve Kargl
@ 2018-12-21 17:35       ` Thomas Koenig
  2018-12-21 18:57         ` Steve Kargl
  0 siblings, 1 reply; 13+ messages in thread
From: Thomas Koenig @ 2018-12-21 17:35 UTC (permalink / raw)
  To: sgk, Janne Blomqvist; +Cc: Fortran List, GCC Patches

Hi Steve,

> No, I'm adding the missing functions to the INTERFACE.

Ah, I see. What I missed was that the function is actually translated
to something else.

So, OK for trunk, and thanks for the patch!

Regards

	Thomas

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

* Re: [PATCH] fortran/69121 -- Make IEEE_SCALB generic
  2018-12-21  9:08   ` Janne Blomqvist
  2018-12-21 15:55     ` Steve Kargl
@ 2018-12-21 17:59     ` Steve Kargl
  2018-12-21 19:11       ` Janne Blomqvist
  1 sibling, 1 reply; 13+ messages in thread
From: Steve Kargl @ 2018-12-21 17:59 UTC (permalink / raw)
  To: Janne Blomqvist; +Cc: Fortran List, GCC Patches

On Fri, Dec 21, 2018 at 11:07:08AM +0200, Janne Blomqvist wrote:
> On Fri, Dec 21, 2018 at 8:22 AM Steve Kargl <
> sgk@troutmask.apl.washington.edu> wrote:
> 
> > On Thu, Dec 20, 2018 at 01:47:39PM -0800, Steve Kargl wrote:
> > > The attached patch has been tested on x86_64-*-freebsd.
> > >
> > > OK to commit?
> > >
> > > 2018-12-20  Steven G. Kargl  <kargl@gcc.gnu.org>
> > >
> > >       PR fortran/69121
> > >       * libgfortran/ieee/ieee_arithmetic.F90: Provide missing functions
> > >       in interface for IEEE_SCALB.
> > >
> > > 2018-12-20  Steven G. Kargl  <kargl@gcc.gnu.org>
> > >
> > >       PR fortran/69121
> > >       * gfortran.dg/ieee/ieee_9.f90: New test.
> >
> > Now, tested on i586-*-freebsd.
> >
> 
> Hi, looks ok for trunk.
> 
> A few questions popped into my mind while looking into this:
> 
> 1) Why are none of the _gfortran_ieee_scalb_X_Y functions mentioned in
> gfortran.map? I guess they should all be there?
> 
> 2) Currently all the intrinsics map to the scalbn{,f,l} builtins. However,
> when the integer argument is of kind int64 or int128 we should instead use
> scalbln{,f,l}. This also applies to other intrinsics that use scalbn under
> the hood.
> 
> To clarify, fixing these is not a prerequisite for accepting the patch (I
> already accepted it), but more like topics for further work.
> 

I forgot to address your had a 2) item above.  ieee_scalb appears
to do the right thing.  FX addressed that with his implementation.
The 2nd argument is always cast to integer after reducing the range
to that of integer(4).

The binary floating point representation for a REAL(16) finite number
is x=f*2**e with f in [0.5,1) and e in [-16059,16384].  scalb(x,n) is
x*2**n, which becomes f*2**e*2**n = f*2**(e+n).  If x is the smallest
positive subnormal number, then n can be at most 32443 to still return 
a finite REAL(16) number.  Any larger value overflows to infinity.
If x is the largest positive finite number, then n can be -32443 to
return the small positive subnormal number.  Any more negative value
of n underflows to zero.  (Note, I could be off-by-one, but that is
just a detail.)

Consider

function foo(x,i)
   use ieee_arithmetic
   real(16) foo, c
   integer(8) i
   print *, ieee_scalb(c, i)
end function foo

-fdump-tree-original gives 
 
D.3853 = *i;
__result_foo = scalbnq (c,
    (integer(kind=4)) MAX_EXPR <MIN_EXPR <D.3853, 2147483647>, -2147483647>);

The range [-32443,32443] is a subset of [-huge(),huge(0)].

-- 
Steve

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

* Re: [PATCH] fortran/69121 -- Make IEEE_SCALB generic
  2018-12-21 17:35       ` Thomas Koenig
@ 2018-12-21 18:57         ` Steve Kargl
  0 siblings, 0 replies; 13+ messages in thread
From: Steve Kargl @ 2018-12-21 18:57 UTC (permalink / raw)
  To: Thomas Koenig; +Cc: Janne Blomqvist, Fortran List, GCC Patches

On Fri, Dec 21, 2018 at 06:31:27PM +0100, Thomas Koenig wrote:
> Hi Steve,
> 
> > No, I'm adding the missing functions to the INTERFACE.
> 
> Ah, I see. What I missed was that the function is actually translated
> to something else.
> 
> So, OK for trunk, and thanks for the patch!
> 

Janne, Thomas, Thanks for the quick review.

I first reported the bug 2 years ago (2016-01-02).  I left
it as low-hanging fruit with the hope that a junior gfortran
hacker would cut his/her teeth on a patch.  Junior hasn't
come along, and with nearly 1000 open PRs, I decided to
fix it.  I took a 5-6 month break from working on gfortran.
When I looked a month ago, there were 1002 open PRs.  With my
recent burst of fixing/closing 16 PRs in December, we're 
still at 991.  Too many PRs, too few hands. :-(

-- 
Steve

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

* Re: [PATCH] fortran/69121 -- Make IEEE_SCALB generic
  2018-12-21 17:59     ` Steve Kargl
@ 2018-12-21 19:11       ` Janne Blomqvist
  2018-12-21 19:33         ` Steve Kargl
  0 siblings, 1 reply; 13+ messages in thread
From: Janne Blomqvist @ 2018-12-21 19:11 UTC (permalink / raw)
  To: Steve Kargl; +Cc: Fortran List, GCC Patches

On Fri, Dec 21, 2018 at 7:59 PM Steve Kargl <
sgk@troutmask.apl.washington.edu> wrote:

> On Fri, Dec 21, 2018 at 11:07:08AM +0200, Janne Blomqvist wrote:
> > On Fri, Dec 21, 2018 at 8:22 AM Steve Kargl <
> > sgk@troutmask.apl.washington.edu> wrote:
> >
> > > On Thu, Dec 20, 2018 at 01:47:39PM -0800, Steve Kargl wrote:
> > > > The attached patch has been tested on x86_64-*-freebsd.
> > > >
> > > > OK to commit?
> > > >
> > > > 2018-12-20  Steven G. Kargl  <kargl@gcc.gnu.org>
> > > >
> > > >       PR fortran/69121
> > > >       * libgfortran/ieee/ieee_arithmetic.F90: Provide missing
> functions
> > > >       in interface for IEEE_SCALB.
> > > >
> > > > 2018-12-20  Steven G. Kargl  <kargl@gcc.gnu.org>
> > > >
> > > >       PR fortran/69121
> > > >       * gfortran.dg/ieee/ieee_9.f90: New test.
> > >
> > > Now, tested on i586-*-freebsd.
> > >
> >
> > Hi, looks ok for trunk.
> >
> > A few questions popped into my mind while looking into this:
> >
> > 1) Why are none of the _gfortran_ieee_scalb_X_Y functions mentioned in
> > gfortran.map? I guess they should all be there?
> >
> > 2) Currently all the intrinsics map to the scalbn{,f,l} builtins.
> However,
> > when the integer argument is of kind int64 or int128 we should instead
> use
> > scalbln{,f,l}. This also applies to other intrinsics that use scalbn
> under
> > the hood.
> >
> > To clarify, fixing these is not a prerequisite for accepting the patch (I
> > already accepted it), but more like topics for further work.
> >
>
> I forgot to address your had a 2) item above.  ieee_scalb appears
> to do the right thing.  FX addressed that with his implementation.
> The 2nd argument is always cast to integer after reducing the range
> to that of integer(4).
>
> The binary floating point representation for a REAL(16) finite number
> is x=f*2**e with f in [0.5,1) and e in [-16059,16384].  scalb(x,n) is
> x*2**n, which becomes f*2**e*2**n = f*2**(e+n).  If x is the smallest
> positive subnormal number, then n can be at most 32443 to still return
> a finite REAL(16) number.  Any larger value overflows to infinity.
> If x is the largest positive finite number, then n can be -32443 to
> return the small positive subnormal number.  Any more negative value
> of n underflows to zero.  (Note, I could be off-by-one, but that is
> just a detail.)
>
> Consider
>
> function foo(x,i)
>    use ieee_arithmetic
>    real(16) foo, c
>    integer(8) i
>    print *, ieee_scalb(c, i)
> end function foo
>
> -fdump-tree-original gives
>
> D.3853 = *i;
> __result_foo = scalbnq (c,
>     (integer(kind=4)) MAX_EXPR <MIN_EXPR <D.3853, 2147483647>,
> -2147483647>);
>
> The range [-32443,32443] is a subset of [-huge(),huge(0)].
>

True. I guess the advantage of scalbln* would be to avoid the MAX/MIN_EXPR
and casting for kind int64.

-- 
Janne Blomqvist

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

* Re: [PATCH] fortran/69121 -- Make IEEE_SCALB generic
  2018-12-21 19:11       ` Janne Blomqvist
@ 2018-12-21 19:33         ` Steve Kargl
  2018-12-21 20:01           ` Joseph Myers
  0 siblings, 1 reply; 13+ messages in thread
From: Steve Kargl @ 2018-12-21 19:33 UTC (permalink / raw)
  To: Janne Blomqvist; +Cc: Fortran List, GCC Patches

On Fri, Dec 21, 2018 at 08:59:04PM +0200, Janne Blomqvist wrote:
> On Fri, Dec 21, 2018 at 7:59 PM Steve Kargl <
> >
> > D.3853 = *i;
> > __result_foo = scalbnq (c,
> >     (integer(kind=4)) MAX_EXPR <MIN_EXPR <D.3853, 2147483647>,
> > -2147483647>);
> >
> > The range [-32443,32443] is a subset of [-huge(),huge(0)].
> >
> 
> True. I guess the advantage of scalbln* would be to avoid the MAX/MIN_EXPR
> and casting for kind int64.
> 

fdlibm-based libm has 

#define NMAX    65536
#define NMIN    -65536
double
scalbln(double x, long n)
{

        return (scalbn(x, (n > NMAX) ? NMAX : (n < NMIN) ? NMIN : (int)n));
}

A search for glibc's libm locates https://tinyurl.com/ybcy8w4t
which is a bit-twiddling routine.  Not sure it's worth the
effort.  Joseph Myers might have an opinion.

-- 
Steve

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

* Re: [PATCH] fortran/69121 -- Make IEEE_SCALB generic
  2018-12-21 19:33         ` Steve Kargl
@ 2018-12-21 20:01           ` Joseph Myers
  2018-12-21 20:29             ` Steve Kargl
  0 siblings, 1 reply; 13+ messages in thread
From: Joseph Myers @ 2018-12-21 20:01 UTC (permalink / raw)
  To: Steve Kargl; +Cc: Janne Blomqvist, Fortran List, GCC Patches

On Fri, 21 Dec 2018, Steve Kargl wrote:

> scalbln(double x, long n)
> {
> 
>         return (scalbn(x, (n > NMAX) ? NMAX : (n < NMIN) ? NMIN : (int)n));
> }
> 
> A search for glibc's libm locates https://tinyurl.com/ybcy8w4t
> which is a bit-twiddling routine.  Not sure it's worth the
> effort.  Joseph Myers might have an opinion.

Such comparisons are needed in the scalbn / scalbln implementations anyway 
to deal with large exponents.  I suppose where there's a suitable scalbln 
implementation, and you don't know if the arguments are within the range 
of int, calling scalbln at least saves code size in the caller and avoids 
duplicating those range checks.

-- 
Joseph S. Myers
joseph@codesourcery.com

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

* Re: [PATCH] fortran/69121 -- Make IEEE_SCALB generic
  2018-12-21 20:01           ` Joseph Myers
@ 2018-12-21 20:29             ` Steve Kargl
  2018-12-24 14:14               ` Sudakshina Das
  0 siblings, 1 reply; 13+ messages in thread
From: Steve Kargl @ 2018-12-21 20:29 UTC (permalink / raw)
  To: Joseph Myers; +Cc: Janne Blomqvist, Fortran List, GCC Patches

On Fri, Dec 21, 2018 at 07:39:45PM +0000, Joseph Myers wrote:
> On Fri, 21 Dec 2018, Steve Kargl wrote:
> 
> > scalbln(double x, long n)
> > {
> > 
> >         return (scalbn(x, (n > NMAX) ? NMAX : (n < NMIN) ? NMIN : (int)n));
> > }
> > 
> > A search for glibc's libm locates https://tinyurl.com/ybcy8w4t
> > which is a bit-twiddling routine.  Not sure it's worth the
> > effort.  Joseph Myers might have an opinion.
> 
> Such comparisons are needed in the scalbn / scalbln implementations anyway 
> to deal with large exponents.  I suppose where there's a suitable scalbln 
> implementation, and you don't know if the arguments are within the range 
> of int, calling scalbln at least saves code size in the caller and avoids 
> duplicating those range checks.
> 

I was thinking along the lines of -ffast-math and whether 
__builtin_scalbn and __builtin_scalbln are then inlined.  
The comparisons may inhibit inlining __builtin_scalbn;
while, if gfortran used __builtin_scalbln, inlining would
occur.

As it is, for

   function foo(x,i)
     use ieee_arithmetic
     real(8) foo, c
     integer(8) i
     foo = ieee_scalb(c, i)
   end function foo

the options -ffast-math -O3 -fdump-tree-optimized give

  <bb 2> [local count: 1073741824]:
  _gfortran_ieee_procedure_entry (&fpstate.0);
  _8 = *i_7(D);
  _1 = MIN_EXPR <_8, 2147483647>;
  _2 = MAX_EXPR <_1, -2147483647>;
  _3 = (integer(kind=4)) _2;
  _4 = __builtin_scalbn (c_9(D), _3);
  _gfortran_ieee_procedure_exit (&fpstate.0);
  fpstate.0 ={v} {CLOBBER};
  return _4;

It seems this could be 

  <bb 2> [local count: 1073741824]:
  _gfortran_ieee_procedure_entry (&fpstate.0);
  _3 = (integer(kind=4)) *i_7(D);
  _4 = __builtin_scalbn (c_9(D), _3);
  _gfortran_ieee_procedure_exit (&fpstate.0);
  fpstate.0 ={v} {CLOBBER};

-- 
Steve

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

* Re: [PATCH] fortran/69121 -- Make IEEE_SCALB generic
  2018-12-21 20:29             ` Steve Kargl
@ 2018-12-24 14:14               ` Sudakshina Das
  2018-12-24 18:05                 ` Steve Kargl
  0 siblings, 1 reply; 13+ messages in thread
From: Sudakshina Das @ 2018-12-24 14:14 UTC (permalink / raw)
  To: sgk, Joseph Myers; +Cc: Janne Blomqvist, Fortran List, GCC Patches, nd

Hi Steve

On 21/12/18 8:01 PM, Steve Kargl wrote:
> On Fri, Dec 21, 2018 at 07:39:45PM +0000, Joseph Myers wrote:
>> On Fri, 21 Dec 2018, Steve Kargl wrote:
>>
>>> scalbln(double x, long n)
>>> {
>>>
>>>          return (scalbn(x, (n > NMAX) ? NMAX : (n < NMIN) ? NMIN : (int)n));
>>> }
>>>
>>> A search for glibc's libm locateshttps://tinyurl.com/ybcy8w4t
>>> which is a bit-twiddling routine.  Not sure it's worth the
>>> effort.  Joseph Myers might have an opinion.
>> Such comparisons are needed in the scalbn / scalbln implementations anyway
>> to deal with large exponents.  I suppose where there's a suitable scalbln
>> implementation, and you don't know if the arguments are within the range
>> of int, calling scalbln at least saves code size in the caller and avoids
>> duplicating those range checks.
>>
> I was thinking along the lines of -ffast-math and whether
> __builtin_scalbn and __builtin_scalbln are then inlined.
> The comparisons may inhibit inlining __builtin_scalbn;
> while, if gfortran used __builtin_scalbln, inlining would
> occur.
>
> As it is, for
>
>     function foo(x,i)
>       use ieee_arithmetic
>       real(8) foo, c
>       integer(8) i
>       foo = ieee_scalb(c, i)
>     end function foo
>
> the options -ffast-math -O3 -fdump-tree-optimized give
>
>    <bb 2> [local count: 1073741824]:
>    _gfortran_ieee_procedure_entry (&fpstate.0);
>    _8 = *i_7(D);
>    _1 = MIN_EXPR <_8, 2147483647>;
>    _2 = MAX_EXPR <_1, -2147483647>;
>    _3 = (integer(kind=4)) _2;
>    _4 = __builtin_scalbn (c_9(D), _3);
>    _gfortran_ieee_procedure_exit (&fpstate.0);
>    fpstate.0 ={v} {CLOBBER};
>    return _4;
>
> It seems this could be
>
>    <bb 2> [local count: 1073741824]:
>    _gfortran_ieee_procedure_entry (&fpstate.0);
>    _3 = (integer(kind=4)) *i_7(D);
>    _4 = __builtin_scalbn (c_9(D), _3
>    _gfortran_ieee_procedure_exit (&fpstate.0);
>    fpstate.0 ={v} {CLOBBER};
>
I am observing your new test pr88328.f90 failing on 
arm-none-linux-gnueabihf:
Excess errors:
/build/src/gcc/gcc/testsuite/gfortran.dg/ieee/ieee_9.f90:20:36: Error: 
Invalid kind for REAL at (1)
/build/src/gcc/gcc/testsuite/gfortran.dg/ieee/ieee_9.f90:35:36: Error: 
Invalid kind for REAL at (1)
/build/src/gcc/gcc/testsuite/gfortran.dg/ieee/ieee_9.f90:50:36: Error: 
Invalid kind for REAL at (1)
/build/src/gcc/gcc/testsuite/gfortran.dg/ieee/ieee_9.f90:65:36: Error: 
Invalid kind for REAL at (1)


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

* Re: [PATCH] fortran/69121 -- Make IEEE_SCALB generic
  2018-12-24 14:14               ` Sudakshina Das
@ 2018-12-24 18:05                 ` Steve Kargl
  0 siblings, 0 replies; 13+ messages in thread
From: Steve Kargl @ 2018-12-24 18:05 UTC (permalink / raw)
  To: Sudakshina Das
  Cc: Joseph Myers, Janne Blomqvist, Fortran List, GCC Patches, nd

On Mon, Dec 24, 2018 at 12:26:46PM +0000, Sudakshina Das wrote:
> >
> I am observing your new test pr88328.f90 failing on 
> arm-none-linux-gnueabihf:
> Excess errors:
> /build/src/gcc/gcc/testsuite/gfortran.dg/ieee/ieee_9.f90:20:36: Error: 
> Invalid kind for REAL at (1)
> /build/src/gcc/gcc/testsuite/gfortran.dg/ieee/ieee_9.f90:35:36: Error: 
> Invalid kind for REAL at (1)
> /build/src/gcc/gcc/testsuite/gfortran.dg/ieee/ieee_9.f90:50:36: Error: 
> Invalid kind for REAL at (1)
> /build/src/gcc/gcc/testsuite/gfortran.dg/ieee/ieee_9.f90:65:36: Error: 
> Invalid kind for REAL at (1)

pr88328.f90 or ieee_9.f90?

In either case, you can XFAIL the testcase for arm-none-linux-gnueabihf.

-- 
Steve

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

end of thread, other threads:[~2018-12-24 17:10 UTC | newest]

Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-12-20 21:53 [PATCH] fortran/69121 -- Make IEEE_SCALB generic Steve Kargl
2018-12-21  6:46 ` Steve Kargl
2018-12-21  9:08   ` Janne Blomqvist
2018-12-21 15:55     ` Steve Kargl
2018-12-21 17:35       ` Thomas Koenig
2018-12-21 18:57         ` Steve Kargl
2018-12-21 17:59     ` Steve Kargl
2018-12-21 19:11       ` Janne Blomqvist
2018-12-21 19:33         ` Steve Kargl
2018-12-21 20:01           ` Joseph Myers
2018-12-21 20:29             ` Steve Kargl
2018-12-24 14:14               ` Sudakshina Das
2018-12-24 18:05                 ` Steve Kargl

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