* [PATCH] Don't call double rint from float powf.
@ 2017-12-12 0:14 Jim Wilson
2017-12-12 3:45 ` Craig Howland
2017-12-12 19:39 ` [PATCH v2] " Jim Wilson
0 siblings, 2 replies; 8+ messages in thread
From: Jim Wilson @ 2017-12-12 0:14 UTC (permalink / raw)
To: newlib; +Cc: Jim Wilson
This is the first patch of two to fix problems with the powf code. I haven't
written the other patch yet.
For targets that have only single float support, such as RISC-V rv32imafc, a
program using powf ends up pulling in soft-float adddf3 and subdf3 routines.
These come from the double rint call, which should be float rintf instead.
This makes a significant difference in the size of the resulting program.
I tested this with a simple testcase, with an rint function that calls abort
to verify that I'm testing the right code path. Compiling with
"gcc -fno-builtin" I get for the patched and unpatched newlib
gamma02:2195$ ls -lt a.out*
-rwxrwxr-x 1 jimw jimw 47012 Dec 11 15:01 a.out
-rwxrwxr-x 1 jimw jimw 75680 Dec 11 14:46 a.out.unpatched
gamma02:2196$
So the result with the patch is about 60% of the size without the patch. The
test program gives the correct result with the patch. I also ran the newlib
make check, which passed with no regression, but I see it doesn't do much
useful.
newlib/
* libm/math/wf_pow.c (powf): Call rintf instead of rint.
---
newlib/libm/math/wf_pow.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/newlib/libm/math/wf_pow.c b/newlib/libm/math/wf_pow.c
index be453558b..4ca0dc79d 100644
--- a/newlib/libm/math/wf_pow.c
+++ b/newlib/libm/math/wf_pow.c
@@ -127,11 +127,11 @@
if (_LIB_VERSION == _SVID_) {
exc.retval = HUGE;
y *= 0.5;
- if(x<0.0&&rint(y)!=y) exc.retval = -HUGE;
+ if(x<0.0&&rintf(y)!=y) exc.retval = -HUGE;
} else {
exc.retval = HUGE_VAL;
y *= 0.5;
- if(x<0.0&&rint(y)!=y) exc.retval = -HUGE_VAL;
+ if(x<0.0&&rintf(y)!=y) exc.retval = -HUGE_VAL;
}
if (_LIB_VERSION == _POSIX_)
errno = ERANGE;
--
2.14.1
Testcase:
#include <stdlib.h>
#include <math.h>
int
main (void)
{
float f = powf (-2.0, -2.0);
if (f != 0.25)
abort ();
float g = powf (-2.0, -3.0);
if (g != -0.125)
abort ();
float a = powf (-2.0, 129);
if (a != -__builtin_inff ())
abort ();
float b = powf (-2.0, 130);
if (b != __builtin_inff ())
abort ();
return 0;
}
double
rint (double x)
{
abort ();
}
^ permalink raw reply [flat|nested] 8+ messages in thread* Re: [PATCH] Don't call double rint from float powf.
2017-12-12 0:14 [PATCH] Don't call double rint from float powf Jim Wilson
@ 2017-12-12 3:45 ` Craig Howland
2017-12-12 12:49 ` Corinna Vinschen
2017-12-12 18:05 ` Jim Wilson
2017-12-12 19:39 ` [PATCH v2] " Jim Wilson
1 sibling, 2 replies; 8+ messages in thread
From: Craig Howland @ 2017-12-12 3:45 UTC (permalink / raw)
To: newlib
On 12/11/2017 07:08 PM, Jim Wilson wrote:
> This is the first patch of two to fix problems with the powf code. I haven't
> written the other patch yet.
>
> For targets that have only single float support, such as RISC-V rv32imafc, a
> program using powf ends up pulling in soft-float adddf3 and subdf3 routines.
> These come from the double rint call, which should be float rintf instead.
> This makes a significant difference in the size of the resulting program.
>
> I tested this with a simple testcase, with an rint function that calls abort
> to verify that I'm testing the right code path. Compiling with
> "gcc -fno-builtin" I get for the patched and unpatched newlib
>
> gamma02:2195$ ls -lt a.out*
> -rwxrwxr-x 1 jimw jimw 47012 Dec 11 15:01 a.out
> -rwxrwxr-x 1 jimw jimw 75680 Dec 11 14:46 a.out.unpatched
> gamma02:2196$
>
> So the result with the patch is about 60% of the size without the patch. The
> test program gives the correct result with the patch. I also ran the newlib
> make check, which passed with no regression, but I see it doesn't do much
> useful.
>
> newlib/
> * libm/math/wf_pow.c (powf): Call rintf instead of rint.
> ---
> newlib/libm/math/wf_pow.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/newlib/libm/math/wf_pow.c b/newlib/libm/math/wf_pow.c
> index be453558b..4ca0dc79d 100644
> --- a/newlib/libm/math/wf_pow.c
> +++ b/newlib/libm/math/wf_pow.c
> @@ -127,11 +127,11 @@
> if (_LIB_VERSION == _SVID_) {
> exc.retval = HUGE;
> y *= 0.5;
> - if(x<0.0&&rint(y)!=y) exc.retval = -HUGE;
> + if(x<0.0&&rintf(y)!=y) exc.retval = -HUGE;
> } else {
> exc.retval = HUGE_VAL;
> y *= 0.5;
> - if(x<0.0&&rint(y)!=y) exc.retval = -HUGE_VAL;
> + if(x<0.0&&rintf(y)!=y) exc.retval = -HUGE_VAL;
> }
> if (_LIB_VERSION == _POSIX_)
> errno = ERANGE;
To be most rigorous, "0.0" on the same lines as the rintf() should be "0.0F" (as
otherwise the comparison strictly should be done in double). (Not addressing
the exact change you are making, but is the same class of fix and are on the
same line of source code.)
^ permalink raw reply [flat|nested] 8+ messages in thread* Re: [PATCH] Don't call double rint from float powf.
2017-12-12 3:45 ` Craig Howland
@ 2017-12-12 12:49 ` Corinna Vinschen
2017-12-12 18:14 ` Jim Wilson
2017-12-12 18:05 ` Jim Wilson
1 sibling, 1 reply; 8+ messages in thread
From: Corinna Vinschen @ 2017-12-12 12:49 UTC (permalink / raw)
To: newlib
[-- Attachment #1: Type: text/plain, Size: 2559 bytes --]
On Dec 11 19:37, Craig Howland wrote:
> On 12/11/2017 07:08 PM, Jim Wilson wrote:
> > This is the first patch of two to fix problems with the powf code. I haven't
> > written the other patch yet.
> >
> > For targets that have only single float support, such as RISC-V rv32imafc, a
> > program using powf ends up pulling in soft-float adddf3 and subdf3 routines.
> > These come from the double rint call, which should be float rintf instead.
> > This makes a significant difference in the size of the resulting program.
> >
> > I tested this with a simple testcase, with an rint function that calls abort
> > to verify that I'm testing the right code path. Compiling with
> > "gcc -fno-builtin" I get for the patched and unpatched newlib
> >
> > gamma02:2195$ ls -lt a.out*
> > -rwxrwxr-x 1 jimw jimw 47012 Dec 11 15:01 a.out
> > -rwxrwxr-x 1 jimw jimw 75680 Dec 11 14:46 a.out.unpatched
> > gamma02:2196$
> >
> > So the result with the patch is about 60% of the size without the patch. The
> > test program gives the correct result with the patch. I also ran the newlib
> > make check, which passed with no regression, but I see it doesn't do much
> > useful.
> >
> > newlib/
> > * libm/math/wf_pow.c (powf): Call rintf instead of rint.
> > ---
> > newlib/libm/math/wf_pow.c | 4 ++--
> > 1 file changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/newlib/libm/math/wf_pow.c b/newlib/libm/math/wf_pow.c
> > index be453558b..4ca0dc79d 100644
> > --- a/newlib/libm/math/wf_pow.c
> > +++ b/newlib/libm/math/wf_pow.c
> > @@ -127,11 +127,11 @@
> > if (_LIB_VERSION == _SVID_) {
> > exc.retval = HUGE;
> > y *= 0.5;
> > - if(x<0.0&&rint(y)!=y) exc.retval = -HUGE;
> > + if(x<0.0&&rintf(y)!=y) exc.retval = -HUGE;
> > } else {
> > exc.retval = HUGE_VAL;
> > y *= 0.5;
> > - if(x<0.0&&rint(y)!=y) exc.retval = -HUGE_VAL;
> > + if(x<0.0&&rintf(y)!=y) exc.retval = -HUGE_VAL;
> > }
> > if (_LIB_VERSION == _POSIX_)
> > errno = ERANGE;
> To be most rigorous, "0.0" on the same lines as the rintf() should be "0.0F"
> (as otherwise the comparison strictly should be done in double). (Not
> addressing the exact change you are making, but is the same class of fix and
> are on the same line of source code.)
Also, HUGE_VAL is double, so it should better be replaced with HUGE_VALF.
Thanks,
Corinna
--
Corinna Vinschen
Cygwin Maintainer
Red Hat
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply [flat|nested] 8+ messages in thread* Re: [PATCH] Don't call double rint from float powf.
2017-12-12 12:49 ` Corinna Vinschen
@ 2017-12-12 18:14 ` Jim Wilson
0 siblings, 0 replies; 8+ messages in thread
From: Jim Wilson @ 2017-12-12 18:14 UTC (permalink / raw)
To: newlib
On 12/12/2017 04:48 AM, Corinna Vinschen wrote:
> On Dec 11 19:37, Craig Howland wrote:
>>> + if(x<0.0&&rintf(y)!=y) exc.retval = -HUGE_VAL;
>>> }
>>> if (_LIB_VERSION == _POSIX_)
>>> errno = ERANGE;
>> To be most rigorous, "0.0" on the same lines as the rintf() should be "0.0F"
>> (as otherwise the comparison strictly should be done in double). (Not
>> addressing the exact change you are making, but is the same class of fix and
>> are on the same line of source code.)
>
> Also, HUGE_VAL is double, so it should better be replaced with HUGE_VALF.
The "struct exception" type uses doubles, so this has to be HUGE_VAL.
Even with my two patches, we still have extendsfdf2 and truncdfsf2
calls, but those will be much harder to get rid of, as we can't change
struct exception without breaking matherr. All of the other soft-float
calls are gone though.
Jim
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] Don't call double rint from float powf.
2017-12-12 3:45 ` Craig Howland
2017-12-12 12:49 ` Corinna Vinschen
@ 2017-12-12 18:05 ` Jim Wilson
2017-12-12 19:00 ` Craig Howland
1 sibling, 1 reply; 8+ messages in thread
From: Jim Wilson @ 2017-12-12 18:05 UTC (permalink / raw)
To: Craig Howland, newlib
On 12/11/2017 04:37 PM, Craig Howland wrote:
> On 12/11/2017 07:08 PM, Jim Wilson wrote:
>> -Â Â Â Â Â Â Â Â Â Â Â Â Â Â if(x<0.0&&rint(y)!=y) exc.retval = -HUGE_VAL;
>> +Â Â Â Â Â Â Â Â Â Â Â Â Â Â if(x<0.0&&rintf(y)!=y) exc.retval = -HUGE_VAL;
>> Â Â Â Â Â Â Â Â Â Â Â Â Â }
>> Â Â Â Â Â Â Â Â Â Â Â Â Â if (_LIB_VERSION == _POSIX_)
>> Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â errno = ERANGE;
> To be most rigorous, "0.0" on the same lines as the rintf() should be
> "0.0F" (as otherwise the comparison strictly should be done in double).
> (Not addressing the exact change you are making, but is the same class
> of fix and are on the same line of source code.)
Any reasonable compiler will get this right. GCC does a float compare
here even if you compile without optimization. So no fix here is
required, though I can redo the patch if you want.
Jim
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] Don't call double rint from float powf.
2017-12-12 18:05 ` Jim Wilson
@ 2017-12-12 19:00 ` Craig Howland
0 siblings, 0 replies; 8+ messages in thread
From: Craig Howland @ 2017-12-12 19:00 UTC (permalink / raw)
To: newlib
On 12/12/2017 01:00 PM, Jim Wilson wrote:
> On 12/11/2017 04:37 PM, Craig Howland wrote:
>> On 12/11/2017 07:08 PM, Jim Wilson wrote:
>>> - if(x<0.0&&rint(y)!=y) exc.retval = -HUGE_VAL;
>>> +Â Â Â Â Â Â Â Â Â Â Â Â Â Â if(x<0.0&&rintf(y)!=y) exc.retval = -HUGE_VAL;
>>> Â Â Â Â Â Â Â Â Â Â Â Â Â }
>>> Â Â Â Â Â Â Â Â Â Â Â Â Â if (_LIB_VERSION == _POSIX_)
>>> Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â errno = ERANGE;
>> To be most rigorous, "0.0" on the same lines as the rintf() should be "0.0F"
>> (as otherwise the comparison strictly should be done in double). (Not
>> addressing the exact change you are making, but is the same class of fix and
>> are on the same line of source code.)
>
> Any reasonable compiler will get this right. GCC does a float compare here
> even if you compile without optimization. So no fix here is required, though
> I can redo the patch if you want.
>
If you don't mind, it would be best that way, as it keeps the source compliant
with the C standard in terms of requiring a float comparison. But it also could
be cleaned up later on, as I expect that there are some others that are the same
way that could also use it.
(It is an interesting thing to consider to decide if GCC is doing the right
thing here, or not. While it knows that it is comparing against 0 and that
converting it from double 0 to float 0 has no runtime side effects, the user
could have purposely done it this way because they wanted to avoid a float
comparison. I do agree that making it a float comparison is a good choice, just
that it could potentially be considered to be not in compliance with the standard.)
^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH v2] Don't call double rint from float powf.
2017-12-12 0:14 [PATCH] Don't call double rint from float powf Jim Wilson
2017-12-12 3:45 ` Craig Howland
@ 2017-12-12 19:39 ` Jim Wilson
2017-12-13 10:35 ` Corinna Vinschen
1 sibling, 1 reply; 8+ messages in thread
From: Jim Wilson @ 2017-12-12 19:39 UTC (permalink / raw)
To: newlib; +Cc: Jim Wilson
Updated patch to use 0.0f in addition to calling rintf.
Tested same way as before, with a testcase that triggers the code and
make check.
OK?
newlib/
* libm/math/wf_pow.c (powf): Call rintf instead of rint. Use 0.0f
for compare.
---
newlib/libm/math/wf_pow.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/newlib/libm/math/wf_pow.c b/newlib/libm/math/wf_pow.c
index be453558b..9a10254bf 100644
--- a/newlib/libm/math/wf_pow.c
+++ b/newlib/libm/math/wf_pow.c
@@ -127,11 +127,11 @@
if (_LIB_VERSION == _SVID_) {
exc.retval = HUGE;
y *= 0.5;
- if(x<0.0&&rint(y)!=y) exc.retval = -HUGE;
+ if(x<0.0f&&rintf(y)!=y) exc.retval = -HUGE;
} else {
exc.retval = HUGE_VAL;
y *= 0.5;
- if(x<0.0&&rint(y)!=y) exc.retval = -HUGE_VAL;
+ if(x<0.0f&&rintf(y)!=y) exc.retval = -HUGE_VAL;
}
if (_LIB_VERSION == _POSIX_)
errno = ERANGE;
--
2.14.1
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2017-12-13 10:07 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-12-12 0:14 [PATCH] Don't call double rint from float powf Jim Wilson
2017-12-12 3:45 ` Craig Howland
2017-12-12 12:49 ` Corinna Vinschen
2017-12-12 18:14 ` Jim Wilson
2017-12-12 18:05 ` Jim Wilson
2017-12-12 19:00 ` Craig Howland
2017-12-12 19:39 ` [PATCH v2] " Jim Wilson
2017-12-13 10:35 ` Corinna Vinschen
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).