g++ vs subnormals in 5.26.0 #15990
Comments
From @jhiSomething changed in g++ 6 as compared with gcc 6 or g++ 5: the handling t/op/sprintf2.t: The problem seems to be the side of $ ./miniperl.gcc -wle 'print unpack("H*",pack("d",0x1.fffffffffffffp-1022))' The behavioural change has been confirmed with both in SUSE Linux (g++ The discussion in the thread "C++ / G++" in One strange thing noticed that if configured with longdoubles, the g++ 6 |
From @jhi
... and the problem is only with the hexadecimal floating point literals: ./miniperl.gcc -wle 'print 0x1.fffffffffffffp-1022' |
From @jkeenanOn Sun, 28 May 2017 06:03:56 GMT, jhi wrote:
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. -- |
|
The RT System itself - Status changed from 'new' to 'open' |
From @jhi
Uh, sorry, I am pretty certain I do no more have the setup anywhere, and
|
From @sisyphusOn Mon, 03 Sep 2018 10:33:33 -0700, jkeenan wrote:
I agree.
I see the same with g++ 7.3.0 on Ubuntu-18.04. 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;' though this works as expected when rewritten as: ~/comp/perl-5.29.2$ ./perl -le 'printf "%a\n", 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;' (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, |
From @iabynOn Tue, Sep 04, 2018 at 07:29:00AM -0700, sisyphus@cpan.org via RT wrote:
The commit which added those comments was mine, although can no longer commit 38c84d6 sprintf2.t: mark TODO bad denorm values under g++ -- |
From @sisyphusOn Tue, 04 Sep 2018 12:48:48 -0700, davem wrote:
Yes, you're right. Cheers, |
From @sisyphusOn Tue, 04 Sep 2018 12:48:48 -0700, davem wrote:
The attached patch to current blead fixes the bug for me. Cheers, |
From @sisyphus0001-toke.c-Cast-I32-to-NV-in-Perl_pow-call.patchFrom 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
|
From @jkeenanOn Wed, 05 Sep 2018 12:20:20 GMT, sisyphus@cpan.org wrote:
Created branch for smoke-testing: smoke-me/jkeenan/sisyphus/131388-subnormals -- |
From @sisyphusOn Wed, 05 Sep 2018 14:17:46 -0700, jkeenan wrote:
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, |
From @jkeenanOn Wed, 05 Sep 2018 23:47:32 GMT, sisyphus@cpan.org wrote:
Just now I reverted that commit and pushed it back to origin in same branch.
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. |
From @jkeenanOn Thu, 06 Sep 2018 02:20:46 GMT, 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. Thank you very much. -- |
From @jhi
*All* the combinations is of course far too large a number, sadly. The basic boring combination is gcc (or clang) on x86-64 on linux Some degrees to consider: - x86-32 - gcc - non-Linux - long double |
From @iabynOn Wed, Sep 05, 2018 at 07:25:55PM -0700, James E Keenan via RT wrote:
The best thing to do would be to convert the TODO condition into a test, so that if this condition arises again it will be quicker to diagnose. e.g. ok($f != 0.0 || $t->[2] eq '0x0p+0', -- |
From @sisyphusOn Wed, 05 Sep 2018 19:25:55 -0700, jkeenan wrote:
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). 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'). It also seemed strange that, having built perl with 'g++ -ansi', any XS modules subsequently installed, will be built without the '-ansi' switch. Cheers, |
From @sisyphusOn Tue, 11 Sep 2018 19:11:02 -0700, sisyphus@cpan.org wrote:
Eventually ... Cheers, |
From @sisyphusOn Tue, 11 Sep 2018 19:11:02 -0700, sisyphus@cpan.org wrote:
On closer inspection I can see that's already happening. 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. Cheers, |
From @sisyphus0001-expand-scope-of-quadmath-tests-in-t-op-sprintf2.t.patchFrom 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
|
From @jkeenanOn Sat, 22 Sep 2018 12:14:40 GMT, sisyphus@cpan.org wrote:
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: ##### ... I got this error: ##### Test 14 in dist/threads/t/exit.t is coded like this: ##### 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. -- |
From @sisyphusOn Sun, 23 Sep 2018 05:37:52 -0700, jkeenan wrote:
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, |
From @sisyphusIt 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. 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, |
Migrated from rt.perl.org#131388 (status was 'open')
Searchable as RT131388$
The text was updated successfully, but these errors were encountered: