Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

g++ vs subnormals in 5.26.0 #15990

Open
p5pRT opened this issue May 28, 2017 · 24 comments
Open

g++ vs subnormals in 5.26.0 #15990

p5pRT opened this issue May 28, 2017 · 24 comments

Comments

@p5pRT
Copy link

p5pRT commented May 28, 2017

Migrated from rt.perl.org#131388 (status was 'open')

Searchable as RT131388$

@p5pRT
Copy link
Author

p5pRT commented May 28, 2017

From @jhi

Something changed in g++ 6 as compared with gcc 6 or g++ 5​: the handling
of subnormal (denormal) floating point values changed so that the Perl
core code parsing (and trying to generate) subnormal numbers does the
"flush-to-zero" instead of retaining the subnormal values.

t/op/sprintf2.t​:
...
ok 1491 - subnormal 3e-324 %.1a 0x1.0p-1074 got 0x1.0p-1074
not ok 1492 - subnormal 0x1.fffffffffffffp-1022 %a
0x1.fffffffffffffp-1022 got 0x0p+0
not ok 1493 - subnormal 0x0.fffffffffffffp-1022 %a
0x1.ffffffffffffep-1023 got 0x0p+0
not ok 1494 - subnormal 0x0.7ffffffffffffp-1022 %a
0x1.ffffffffffffcp-1024 got 0x0p+0
not ok 1495 - subnormal 0x0.3ffffffffffffp-1022 %a
0x1.ffffffffffff8p-1025 got 0x0p+0
not ok 1496 - subnormal 0x0.1ffffffffffffp-1022 %a
0x1.ffffffffffffp-1026 got 0x0p+0
not ok 1497 - subnormal 0x0.0ffffffffffffp-1022 %a
0x1.fffffffffffep-1027 got 0x0p+0
ok 1498
...

The problem seems to be the side of
parsing the hexfp literals, so toke.c Perl_scan_num(), not on the
sv.c sprintf side.

$ ./miniperl.gcc -wle 'print unpack("H*",pack("d",0x1.fffffffffffffp-1022))'
ffffffffffff1f00
$ ./miniperl.g++ -wle 'print unpack("H*",pack("d",0x1.fffffffffffffp-1022))'
0000000000000000

The behavioural change has been confirmed with both in SUSE Linux (g++
(SUSE Linux) 6.3.1 20170202 [gcc-6-branch revision 245119])
and in OS X (with gcc version 6.3.0 (MacPorts gcc6 6.3.0_2)

The discussion in the thread "C++ / G++" in
https://www.nntp.perl.org/group/perl.perl5.porters/2017/05/msg244361.html
or https://marc.info/?l=perl5-porters&m=149475718717687&w=2

One strange thing noticed that if configured with longdoubles, the g++ 6
build does work correctly​:
https://www.nntp.perl.org/group/perl.perl5.porters/2017/05/msg244411.html
or https://marc.info/?l=perl5-porters&m=149495399107618&w=2

@p5pRT
Copy link
Author

p5pRT commented May 28, 2017

From @jhi

$ ./miniperl.gcc -wle 'print unpack("H*",pack("d",0x1.fffffffffffffp-1022))'
ffffffffffff1f00
$ ./miniperl.g++ -wle 'print unpack("H*",pack("d",0x1.fffffffffffffp-1022))'
0000000000000000

... and the problem is only with the hexadecimal floating point literals​:

./miniperl.gcc -wle 'print 0x1.fffffffffffffp-1022'
4.4501477170144e-308
./miniperl.g++ -wle 'printf "%a\n", 4.4501477170144e-308'
0x1.ffffffffffff5p-1022

@p5pRT
Copy link
Author

p5pRT commented Sep 3, 2018

From @jkeenan

On Sun, 28 May 2017 06​:03​:56 GMT, jhi wrote​:

Something changed in g++ 6 as compared with gcc 6 or g++ 5​: the handling
of subnormal (denormal) floating point values changed so that the Perl
core code parsing (and trying to generate) subnormal numbers does the
"flush-to-zero" instead of retaining the subnormal values.

t/op/sprintf2.t​:
...
ok 1491 - subnormal 3e-324 %.1a 0x1.0p-1074 got 0x1.0p-1074
not ok 1492 - subnormal 0x1.fffffffffffffp-1022 %a
0x1.fffffffffffffp-1022 got 0x0p+0
not ok 1493 - subnormal 0x0.fffffffffffffp-1022 %a
0x1.ffffffffffffep-1023 got 0x0p+0
not ok 1494 - subnormal 0x0.7ffffffffffffp-1022 %a
0x1.ffffffffffffcp-1024 got 0x0p+0
not ok 1495 - subnormal 0x0.3ffffffffffffp-1022 %a
0x1.ffffffffffff8p-1025 got 0x0p+0
not ok 1496 - subnormal 0x0.1ffffffffffffp-1022 %a
0x1.ffffffffffffp-1026 got 0x0p+0
not ok 1497 - subnormal 0x0.0ffffffffffffp-1022 %a
0x1.fffffffffffep-1027 got 0x0p+0
ok 1498
...

The problem seems to be the side of
parsing the hexfp literals, so toke.c Perl_scan_num(), not on the
sv.c sprintf side.

$ ./miniperl.gcc -wle 'print unpack("H*",pack("d",0x1.fffffffffffffp-1022))'
ffffffffffff1f00
$ ./miniperl.g++ -wle 'print unpack("H*",pack("d",0x1.fffffffffffffp-1022))'
0000000000000000

The behavioural change has been confirmed with both in SUSE Linux (g++
(SUSE Linux) 6.3.1 20170202 [gcc-6-branch revision 245119])
and in OS X (with gcc version 6.3.0 (MacPorts gcc6 6.3.0_2)

The discussion in the thread "C++ / G++" in
https://www.nntp.perl.org/group/perl.perl5.porters/2017/05/msg244361.html
or https://marc.info/?l=perl5-porters&m=149475718717687&w=2

One strange thing noticed that if configured with longdoubles, the g++ 6
build does work correctly​:
https://www.nntp.perl.org/group/perl.perl5.porters/2017/05/msg244411.html
or https://marc.info/?l=perl5-porters&m=149495399107618&w=2

Jarkko, could you supply (at least) the Configure args you used to build the perl used in these tests? Or preferably attach the output of "perl -V"?

We should try to reproduce with still newer versions of g++.

Thank you very much.

--
James E Keenan (jkeenan@​cpan.org)

@p5pRT
Copy link
Author

p5pRT commented Sep 3, 2018

The RT System itself - Status changed from 'new' to 'open'

@p5pRT
Copy link
Author

p5pRT commented Sep 3, 2018

From @jhi

build does work correctly​:
https://www.nntp.perl.org/group/perl.perl5.porters/2017/05/msg244411.html
or https://marc.info/?l=perl5-porters&m=149495399107618&w=2

Jarkko, could you supply (at least) the Configure args you used to build the perl used in these tests? Or preferably attach the output of "perl -V"?

Uh, sorry, I am pretty certain I do no more have the setup anywhere, and
my builds are ephemeral. But if I go by my usual reflexes the
invocation of Configure has been "sh Configure -des -Dcc=g++".

We should try to reproduce with still newer versions of g++.

Thank you very much.

@p5pRT
Copy link
Author

p5pRT commented Sep 4, 2018

From @sisyphus

On Mon, 03 Sep 2018 10​:33​:33 -0700, jkeenan wrote​:

On Sun, 28 May 2017 06​:03​:56 GMT, jhi wrote​:

The problem seems to be the side of
parsing the hexfp literals, so toke.c Perl_scan_num(), not on the
sv.c sprintf side.

I agree.
I certainly haven't been able to reproduce the behaviour in any C scripts compiled using g++.

We should try to reproduce with still newer versions of g++.

I see the same with g++ 7.3.0 on Ubuntu-18.04.
And it's happening on 5.28.0 and 5.29.2, too, though the problem has been hidden away (via TODO) since sometime prior to the release of 5.28.0 - so that the failures are no longer reported by 'make test'.

Comments in op/sprintf2.t claim that "pow(2.0, -1074) returns 0", but I couldn't find anything to substantiate that claim.

It's not just subnormals that are affected, either​:

~/comp/perl-5.29.2$ ./perl -le 'printf "%a\n", 0x1.0p-1020;'
0x0p+0

though this works as expected when rewritten as​:

~/comp/perl-5.29.2$ ./perl -le 'printf "%a\n", 0x1p-1020;'
0x1p-1020

And it's not limited to g++ builds, either. I found a few subnormal values where a gcc-7.3.0 build of 5.29.2 on the same machine (and a gcc-8.1.0 build of 5.29.2 on Windows 7) were also behaving improperly - eg​:

$ perl -le 'printf "%a\n", 0x1.0p-1074;'
0x0p+0
$ perl -le 'printf "%a\n", 0x1p-1074;'
0x1p-1074

(Same type of behaviour for exponents -1071, -1072, and -1073 on the same 2 machines.)

I Configured the g++ builds with "-des -Dcc=g++" (and additionally with "-Dusedevel" for 5.29.2).

Cheers,
Rob

@p5pRT
Copy link
Author

p5pRT commented Sep 4, 2018

From @iabyn

On Tue, Sep 04, 2018 at 07​:29​:00AM -0700, sisyphus@​cpan.org via RT wrote​:

Comments in op/sprintf2.t claim that "pow(2.0, -1074) returns 0", but I couldn't find anything to substantiate that claim.

The commit which added those comments was mine, although can no longer
remember the details​:

commit 38c84d6
Author​: David Mitchell <davem@​iabyn.com>
AuthorDate​: Tue May 1 15​:28​:49 2018 +0100
Commit​: David Mitchell <davem@​iabyn.com>
CommitDate​: Tue May 1 15​:52​:55 2018 +0100

  sprintf2.t​: mark TODO bad denorm values under g++
 
  Some t/op/sprintf2.t tests were failing under g++. This is due the perl
  toker interpreting very small literal hex floating pointers as 0 rather
  than as a subnormal value.
 
  For example​:
 
  perl -le'print "bad" if 0x1.fffffffffffffp-1022 == 0.0'
 
  This breaks some of the sprintf2.t tests, so mark them TODO them if the
  literal value evaluates to zero.
 
  Note that this is a bug in the toker/g++/glibc rather than sprintf.
 
  The issue is due to the use of pow() in scan_num()​:
 
  under gcc and plain g++, pow(2.0, -1074) returns the smallest denorm
  number; however, under 'g++ -ansi', it returns 0.0.

--
I thought I was wrong once, but I was mistaken.

@p5pRT
Copy link
Author

p5pRT commented Sep 5, 2018

From @sisyphus

On Tue, 04 Sep 2018 12​:48​:48 -0700, davem wrote​:

under gcc and plain g++, pow(2.0, -1074) returns the smallest denorm
number; however, under 'g++ -ansi', it returns 0.0.

Yes, you're right.
I thought I had checked with the ansi switch last night, but apparently I hadn't.

Cheers,
Rob

@p5pRT
Copy link
Author

p5pRT commented Sep 5, 2018

From @sisyphus

On Tue, 04 Sep 2018 12​:48​:48 -0700, davem wrote​:

The issue is due to the use of pow() in scan_num()

The attached patch to current blead fixes the bug for me.

Cheers,
Rob

@p5pRT
Copy link
Author

p5pRT commented Sep 5, 2018

From @sisyphus

0001-toke.c-Cast-I32-to-NV-in-Perl_pow-call.patch
From 9d00f794432f827520b5db60b8a12d897248d1cf Mon Sep 17 00:00:00 2001
From: sisyphus <sisyphus1@optusnet.com.au>
Date: Wed, 5 Sep 2018 21:36:29 +1000
Subject: [PATCH] toke.c - Cast I32 to NV in Perl_pow() call

---
 toke.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/toke.c b/toke.c
index 24e614fd50..191a1ce8b9 100644
--- a/toke.c
+++ b/toke.c
@@ -11221,7 +11221,7 @@ Perl_scan_num(pTHX_ const char *start, YYSTYPE* lvalp)
 #ifdef HEXFP_UQUAD
                         hexfp_exp -= hexfp_frac_bits;
 #endif
-                        hexfp_mult = Perl_pow(2.0, hexfp_exp);
+                        hexfp_mult = Perl_pow(2.0, (NV)hexfp_exp);
                         hexfp = TRUE;
                         goto decimal;
                     }
-- 
2.17.1

@p5pRT
Copy link
Author

p5pRT commented Sep 5, 2018

From @jkeenan

On Wed, 05 Sep 2018 12​:20​:20 GMT, sisyphus@​cpan.org wrote​:

On Tue, 04 Sep 2018 12​:48​:48 -0700, davem wrote​:

The issue is due to the use of pow() in scan_num()

The attached patch to current blead fixes the bug for me.

Cheers,
Rob

Created branch for smoke-testing​:

smoke-me/jkeenan/sisyphus/131388-subnormals

--
James E Keenan (jkeenan@​cpan.org)

@p5pRT
Copy link
Author

p5pRT commented Sep 5, 2018

From @sisyphus

On Wed, 05 Sep 2018 14​:17​:46 -0700, jkeenan wrote​:

Created branch for smoke-testing​:

smoke-me/jkeenan/sisyphus/131388-subnormals

If the patch is successful, we will also need to undo Dave's commit (38c84d6) to sprintf2.t that made the failing tests "TODO".

In the one report currently available on the new smoke-me branch, I see the line "Test todo-passed​:" with no entries following it - so I'm guessing that will list any TODOs that failed.

Are g++ builds being smoked ?

Cheers,
Rob

@p5pRT
Copy link
Author

p5pRT commented Sep 6, 2018

From @jkeenan

On Wed, 05 Sep 2018 23​:47​:32 GMT, sisyphus@​cpan.org wrote​:

On Wed, 05 Sep 2018 14​:17​:46 -0700, jkeenan wrote​:

Created branch for smoke-testing​:

smoke-me/jkeenan/sisyphus/131388-subnormals

If the patch is successful, we will also need to undo Dave's commit
(38c84d6) to sprintf2.t that made the
failing tests "TODO".

Just now I reverted that commit and pushed it back to origin in same branch.

In the one report currently available on the new smoke-me branch, I
see the line "Test todo-passed​:" with no entries following it - so I'm
guessing that will list any TODOs that failed.

Are g++ builds being smoked ?

FBOW, there is no centralized schema/matrix of platforms/C-compilers being smoked. All smoke testing rigs are maintained by individual volunteers; none (any longer) by perl.org or perl5-porters. So it depends on what individual testers have had the time and energe to set up.

My recommendation​:

1. Based on the discussion in this RT and in any other relevant places on list, make a list of platform/C-compiler combinations which need to be tested to thoroughly exercise the branch.

2. After you've made that list -- not before, which would introduce a potential bias -- go to http​://perl.develop-help.com/?b=smoke-me%2Fjkeenan%2Fsisyphus%2F131388-subnormals and review the results.

3. Then make a list of platform/C-compiler combos which remain to be tested. Post that back in this RT. We'll see what we can do to accommodate you and the needs of the RT.

Thank you very much.
--
James E Keenan (jkeenan@​cpan.org)

@p5pRT
Copy link
Author

p5pRT commented Sep 6, 2018

From @jkeenan

On Thu, 06 Sep 2018 02​:20​:46 GMT, jkeenan wrote​:

On Wed, 05 Sep 2018 23​:47​:32 GMT, sisyphus@​cpan.org wrote​:

On Wed, 05 Sep 2018 14​:17​:46 -0700, jkeenan wrote​:

Created branch for smoke-testing​:

smoke-me/jkeenan/sisyphus/131388-subnormals

If the patch is successful, we will also need to undo Dave's commit
(38c84d6) to sprintf2.t that made
the
failing tests "TODO".

Just now I reverted that commit and pushed it back to origin in same
branch.

In the one report currently available on the new smoke-me branch, I
see the line "Test todo-passed​:" with no entries following it - so
I'm
guessing that will list any TODOs that failed.

Are g++ builds being smoked ?

FBOW, there is no centralized schema/matrix of platforms/C-compilers
being smoked. All smoke testing rigs are maintained by individual
volunteers; none (any longer) by perl.org or perl5-porters. So it
depends on what individual testers have had the time and energe to set
up.

My recommendation​:

1. Based on the discussion in this RT and in any other relevant places
on list, make a list of platform/C-compiler combinations which need to
be tested to thoroughly exercise the branch.

2. After you've made that list -- not before, which would introduce a
potential bias -- go to http​://perl.develop-help.com/?b=smoke-
me%2Fjkeenan%2Fsisyphus%2F131388-subnormals and review the results.

3. Then make a list of platform/C-compiler combos which remain to be
tested. Post that back in this RT. We'll see what we can do to
accommodate you and the needs of the RT.

Thank you very much.

One other consideration​: Your patch added no new tests, though we just reverted the TODO-ing of some older tests. It may be the case -- I haven't really followed the details of this RT that closely -- that to fully resolve the problem we need to write *new* tests. If so, we should add them to this branch and do additional smoke testing.

Thank you very much.

--
James E Keenan (jkeenan@​cpan.org)

@p5pRT
Copy link
Author

p5pRT commented Sep 6, 2018

From @jhi

Are g++ builds being smoked ?

FBOW, there is no centralized schema/matrix of platforms/C-compilers being smoked. All smoke testing rigs are maintained by individual volunteers; none (any longer) by perl.org or perl5-porters. So it depends on what individual testers have had the time and energe to set up.

My recommendation​:

1. Based on the discussion in this RT and in any other relevant places on list, make a list of platform/C-compiler combinations which need to be tested to thoroughly exercise the branch.

*All* the combinations is of course far too large a number, sadly.
Also, I don't think this discussion should be limited to this one
or any one single branch. In general, I think doing first some varying
on the platform, and then some varying on the compiler, would be good.

The basic boring combination is gcc (or clang) on x86-64 on linux
(and therefore glibc). Anything beyond that would be very useful.

Some degrees to consider​:

- x86-32
- non-x86
- bigendian
- some CPU with stricter alignment than x86

- gcc
- clang
- non-gcc-compatible-cc
- C++ compiler
- cross-compiler

- non-Linux
- non-UNIX
- non-UNIX-other-than-windows

- long double
- threads

@p5pRT
Copy link
Author

p5pRT commented Sep 6, 2018

From @iabyn

On Wed, Sep 05, 2018 at 07​:25​:55PM -0700, James E Keenan via RT wrote​:

One other consideration​: Your patch added no new tests, though we just
reverted the TODO-ing of some older tests. It may be the case -- I
haven't really followed the details of this RT that closely -- that to
fully resolve the problem we need to write *new* tests. If so, we
should add them to this branch and do additional smoke testing.

The best thing to do would be to convert the TODO condition into a test,
i.e.
  if $f == 0.0 && $t->[2] ne '0x0p+0';

so that if this condition arises again it will be quicker to diagnose.

e.g.

  ok($f != 0.0 || $t->[2] eq '0x0p+0',
  "denorm value shouldn't be zero​: $t->[0]");

--
"I do not resent criticism, even when, for the sake of emphasis,
it parts for the time with reality".
  -- Winston Churchill, House of Commons, 22nd Jan 1941.

@p5pRT
Copy link
Author

p5pRT commented Sep 12, 2018

From @sisyphus

On Wed, 05 Sep 2018 19​:25​:55 -0700, jkeenan wrote​:

One other consideration​: Your patch added no new tests, though we
just reverted the TODO-ing of some older tests. It may be the case --
I haven't really followed the details of this RT that closely -- that
to fully resolve the problem we need to write *new* tests. If so, we
should add them to this branch and do additional smoke testing.

I suppose, to be thorough, we should also be performing similar tests for subnormal NV values on -Duselongdouble and -Dusequadmath builds.

I'll work on patching the blead version of sprintf2.t to do this - and to also incorporate Dave's suggestion re converting the TODO into actual tests.

Attached is try.c which, when built with 'g++ -ansi' demonstrates the problem - and also demonstrates that the cast to double fixes the issue.

I asked about the validity of the way that 'g++ -ansi' handled that script on the gcc-help mailing list (see https://gcc.gnu.org/ml/gcc-help/2018-09/msg00031.html).
The responses were not exactly definitive, but I gathered it was felt that there was no immediate need to alter the way that g++ was doing anything.

As a result of that thread, I did begin to wonder why it is that g++ builds of perl invoke the '-ansi' switch (which is actually equivalent to '-std=c++89').
Is this setting something that's easily configured differently ?

It also seemed strange that, having built perl with 'g++ -ansi', any XS modules subsequently installed, will be built without the '-ansi' switch.

Cheers,
Rob

@p5pRT
Copy link
Author

p5pRT commented Sep 12, 2018

From @sisyphus

On Tue, 11 Sep 2018 19​:11​:02 -0700, sisyphus@​cpan.org wrote​:

Attached is try.c which, when built with 'g++ -ansi' demonstrates the
problem - and also demonstrates that the cast to double fixes the
issue.

Eventually ...

Cheers,
Rob

@p5pRT
Copy link
Author

p5pRT commented Sep 12, 2018

From @sisyphus

try.c

@p5pRT
Copy link
Author

p5pRT commented Sep 22, 2018

From @sisyphus

On Tue, 11 Sep 2018 19​:11​:02 -0700, sisyphus@​cpan.org wrote​:

I suppose, to be thorough, we should also be performing similar tests
for subnormal NV values on -Duselongdouble and -Dusequadmath builds.

On closer inspection I can see that's already happening.
In fact, I don't see any need to make any alteration to the version of t/op/sprintf2.t that is currently in the smoke-me/jkeenan/sisyphus/131388-subnormals branch ... except, perhaps, for one unrelated observation​:

At line 897 of smoke-me/jkeenan/sisyphus/131388-subnormals/t/op/sprintf2.t, we begin 7 tests on usequadmath builds in response to ticket #128843.
I believe those tests could also be applied to uselongdouble builds where the long double conforms to the IEEE 754 standard.
I'm not bothered one way or the other about this, and I don't have access to a system that provides such long doubles so I can't test it.
But I'm attaching the patch (against smoke-me/jkeenan/sisyphus/131388-subnormals/t/op/sprintf2.t) for review, anyway.

Cheers,
Rob

@p5pRT
Copy link
Author

p5pRT commented Sep 22, 2018

From @sisyphus

0001-expand-scope-of-quadmath-tests-in-t-op-sprintf2.t.patch
From fd3dce7e0d0640f397cf687b00f8d3e4d58aacc0 Mon Sep 17 00:00:00 2001
From: sisyphus <sisyphus1@optusnet.com.au>
Date: Sat, 22 Sep 2018 22:08:17 +1000
Subject: [PATCH] expand scope of quadmath tests in t/op/sprintf2.t

---
 t/op/sprintf2.t | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/t/op/sprintf2.t b/t/op/sprintf2.t
index 5fb7597..6d498db 100644
--- a/t/op/sprintf2.t
+++ b/t/op/sprintf2.t
@@ -896,7 +896,9 @@ SKIP: {
 
 # quadmath tests for rt.perl.org #128843
 SKIP: {
-    skip "need quadmath", 7, unless $Config{usequadmath};
+    skip "need IEEE 754 NV", 7,
+          unless ($Config{usequadmath} ||
+          ($Config{uselongdouble} && $Config{longdblkind} > 0 && $Config{longdblkind} < 3));
 
     is(sprintf("%a", eval '0x1p-16382'), '0x1p-16382');  # last normal
 
-- 
2.1.4

@p5pRT
Copy link
Author

p5pRT commented Sep 23, 2018

From @jkeenan

On Sat, 22 Sep 2018 12​:14​:40 GMT, sisyphus@​cpan.org wrote​:

On Tue, 11 Sep 2018 19​:11​:02 -0700, sisyphus@​cpan.org wrote​:

I suppose, to be thorough, we should also be performing similar tests
for subnormal NV values on -Duselongdouble and -Dusequadmath builds.

On closer inspection I can see that's already happening.
In fact, I don't see any need to make any alteration to the version of
t/op/sprintf2.t that is currently in the smoke-
me/jkeenan/sisyphus/131388-subnormals branch ... except, perhaps, for
one unrelated observation​:

At line 897 of smoke-me/jkeenan/sisyphus/131388-
subnormals/t/op/sprintf2.t, we begin 7 tests on usequadmath builds in
response to ticket #128843.
I believe those tests could also be applied to uselongdouble builds
where the long double conforms to the IEEE 754 standard.
I'm not bothered one way or the other about this, and I don't have
access to a system that provides such long doubles so I can't test it.
But I'm attaching the patch (against smoke-me/jkeenan/sisyphus/131388-
subnormals/t/op/sprintf2.t) for review, anyway.

Cheers,
Rob

Last night I applied your patch to the smoke-me/jkeenan/sisyphus/131388-subnormals branch and pushed it to the origin for smoke-testing.

Most smoke-test results are good, but I got one failure on my my own FreeBSD-11 smoke-testing rig. See​: http​://perl5.test-smoke.org/report/70990. Where the configuration was​:

#####
-des -Dusedevel -Duseithreads -Doptimize=-O2 -pipe -fstack-protector -fno-strict-aliasing -Dcc=g++ -Dusemorebits -DDEBUGGING
#####

... I got this error​:

#####
Test failures​:
~~ ../dist/threads/t/exit.t .................................... FAILED 14
  [perlio] -Duseithreads -Doptimize="-O2 -pipe -fstack-protector -fno-strict-aliasing" -Dcc="g++" -Dusemorebits DEBUGGING
#####

Test 14 in dist/threads/t/exit.t is coded like this​:

#####
$out = run_perl(prog => 'use threads 2.21 qw(exit thread_only);' .
  'threads->create(sub {' .
  ' threads->set_thread_exit_only(0);' .
  ' exit(99);' .
  '});' .
  'sleep(1);' .
  'exit(86);',
  nolib => ($ENV{PERL_CORE}) ? 0 : 1,
  switches => ($ENV{PERL_CORE}) ? [] : [ '-Mblib' ],
  stderr => 1);
{
  local $TODO = 'VMS exit semantics not like POSIX exit semantics' if $^O eq 'VMS';
  is($?>>8, 99, "set_thread_exit_only(0)");
}
#####

I then went to my git checkout on the same machine, checked out this branch, built perl with the same configuration and ran the test file through the harness. It PASSed.

So the test failure in the smoke-test may be a transient failure due to resource limitations. ISTR seeing such failures in that file previously.

Thank you very much.

--
James E Keenan (jkeenan@​cpan.org)

@p5pRT
Copy link
Author

p5pRT commented Sep 23, 2018

From @sisyphus

On Sun, 23 Sep 2018 05​:37​:52 -0700, jkeenan wrote​:

So the test failure in the smoke-test may be a transient failure due
to resource limitations.

I don't know what would have caused that failure, and I'll happily go along with "resource limitations".

Note that the patch was to t/op/sprintf2.t and that the only failures we could therefore attribute to it would have to be t/op/sprintf2.t failures.

I don't think there are any smokers where $Config{longdblkind} is either 1 or 2, so I don't think my change to t/op/sprintf2.t will make any difference to them.

Hopefully I'm wrong about that - I'd be delighted to learn that there's at least 1 smoker out there that has the quadruple precision, 128-bit, IEEE 754 long double where $Config{longdblkind} is being set correctly to 1 or 2.

The various $Config{longdblkind} values are explained in the Config.pm documentation.

Cheers,
Rob

@p5pRT
Copy link
Author

p5pRT commented Sep 26, 2018

From @sisyphus

It occurs to me that the toke.c patch should also include a comment that the explicit cast to NV is needed for some g++ builds.
Without that comment, it's possible that someone will remove that explicit cast in future.

If the fix of casting to NV is deemed unsatisfactory, I should add that the bug could probably also be fixed in toke.c by replacing "Perl_pow(2.0,hexfp_exp)" with "Perl_ldexp(1.0,hexfp_exp)" though this would require another round of smoke testing.

Cheers,
Rob

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants