Perl / perl5

🐪 The Perl programming language
https://dev.perl.org/perl5/
Other
1.92k stars 549 forks source link

text->float conversion gives wrong answer #8730

Closed p5pRT closed 5 years ago

p5pRT commented 17 years ago

Migrated from rt.perl.org#41202 (status was 'resolved')

Searchable as RT41202$

p5pRT commented 17 years ago

From zefram@fysh.org

Created by zefram@fysh.org

$ perl -lwe 'printf "%f\n"\, 1180591620717411303424.0' 1180591620717411172352.000000 $

The value that I put in the source is exactly 2^70. This is trivially representable as a native float. The value that is printed out is 2^70-2^17\, which is the *next lower* floating-point value. (The significand has 52 fractional bits; this value has exponent 69 and all significand bits set.) 2^70-2^17 itself converts just fine​:

$ perl -lwe 'printf "%f\n"\, 1180591620717411172352.0' 1180591620717411172352.000000 $

Using that I can construct 2^70\, which converts back to text normally​:

$ perl -lwe 'printf "%f\n"\, 1180591620717411172352.0+131072.0' 1180591620717411303424.000000 $

For further evidence that it is the text->float conversion\, and not float->text\, that is in error\, one can use my Data​::Float module to display the actual bit pattern​:

$ perl -MData​::Float=float_hex -lwe 'print float_hex(1180591620717411303424.0)' +0x1.fffffffffffffp+69 $ perl -MData​::Float=float_hex -lwe 'print float_hex(1180591620717411172352.0+131072.0)' +0x1.0000000000000p+70 $

The same conversion fault occurs with runtime string->number conversion also.

This fault does not occur with other programs doing this conversion on the same machine​:

zsh% echo $((1180591620717411303424.0 == 1180591620717411172352.0)) 0 zsh% echo $((1180591620717411303424.0-131072.0 == 1180591620717411172352.0)) 1 zsh%

Perl Info ``` Flags: category=core severity=medium Site configuration information for perl v5.8.4: Configured by Debian Project at Wed May 10 04:14:05 UTC 2006. Summary of my perl5 (revision 5 version 8 subversion 4) configuration: Platform: osname=linux, osvers=2.6.15.6, archname=i386-linux-thread-multi uname='linux ernie 2.6.15.6 #1 thu mar 16 13:11:55 est 2006 i686 gnulinux ' config_args='-Dusethreads -Duselargefiles -Dccflags=-DDEBIAN -Dcccdlflags=-fPIC -Darchname=i386-linux -Dprefix=/usr -Dprivlib=/usr/share/perl/5.8 -Darchlib=/usr/lib/perl/5.8 -Dvendorprefix=/usr -Dvendorlib=/usr/share/perl5 -Dvendorarch=/usr/lib/perl5 -Dsiteprefix=/usr/local -Dsitelib=/usr/local/share/perl/5.8.4 -Dsitearch=/usr/local/lib/perl/5.8.4 -Dman1dir=/usr/share/man/man1 -Dman3dir=/usr/share/man/man3 -Dsiteman1dir=/usr/local/man/man1 -Dsiteman3dir=/usr/local/man/man3 -Dman1ext=1 -Dman3ext=3perl -Dpager=/usr/bin/sensible-pager -Uafs -Ud_csh -Uusesfio -Uusenm -Duseshrplib -Dlibperl=libperl.so.5.8.4 -Dd_dosuid -des' hint=recommended, useposix=true, d_sigaction=define usethreads=define use5005threads=undef useithreads=define usemultiplicity=define useperlio=define d_sfio=undef uselargefiles=define usesocks=undef use64bitint=undef use64bitall=undef uselongdouble=undef usemymalloc=n, bincompat5005=undef Compiler: cc='cc', ccflags ='-D_REENTRANT -D_GNU_SOURCE -DTHREADS_HAVE_PIDS -DDEBIAN -fno-strict-aliasing -I/usr/local/include -D_LARGEFILE_SOURCE -D_FILE_OFFSET_BITS=64', optimize='-O2', cppflags='-D_REENTRANT -D_GNU_SOURCE -DTHREADS_HAVE_PIDS -DDEBIAN -fno-strict-aliasing -I/usr/local/include' ccversion='', gccversion='3.3.5 (Debian 1:3.3.5-13)', gccosandvers='' intsize=4, longsize=4, ptrsize=4, doublesize=8, byteorder=1234 d_longlong=define, longlongsize=8, d_longdbl=define, longdblsize=12 ivtype='long', ivsize=4, nvtype='double', nvsize=8, Off_t='off_t', lseeksize=8 alignbytes=4, prototype=define Linker and Libraries: ld='cc', ldflags =' -L/usr/local/lib' libpth=/usr/local/lib /lib /usr/lib libs=-lgdbm -lgdbm_compat -ldb -ldl -lm -lpthread -lc -lcrypt perllibs=-ldl -lm -lpthread -lc -lcrypt libc=/lib/libc-2.3.2.so, so=so, useshrplib=true, libperl=libperl.so.5.8.4 gnulibc_version='2.3.2' Dynamic Linking: dlsrc=dl_dlopen.xs, dlext=so, d_dlsymun=undef, ccdlflags='-Wl,-E' cccdlflags='-fPIC', lddlflags='-shared -L/usr/local/lib' Locally applied patches: @INC for perl v5.8.4: /etc/perl /usr/local/lib/perl/5.8.4 /usr/local/share/perl/5.8.4 /usr/lib/perl5 /usr/share/perl5 /usr/lib/perl/5.8 /usr/share/perl/5.8 /usr/local/lib/site_perl . Environment for perl v5.8.4: HOME=/home/zefram LANG (unset) LANGUAGE (unset) LD_LIBRARY_PATH (unset) LOGDIR (unset) PATH=/home/zefram/pub/i686-pc-linux-gnu/bin:/home/zefram/pub/common/bin:/usr/bin:/usr/X11R6/bin:/bin:/usr/local/bin:/usr/games PERL_BADLANG (unset) SHELL=/usr/bin/zsh ```
p5pRT commented 13 years ago

From @cpansprout

On Sun Jan 07 10​:45​:52 2007\, zefram@​fysh.org wrote​:

$ perl -lwe 'printf "%f\n"\, 1180591620717411303424.0' 1180591620717411172352.000000 $

The value that I put in the source is exactly 2^70.

Perl’s atof implementation (c2988b20) multiplies by ten every time it scans a new digit. This results in loss of precision. What is the best way to tell whether an NV differs from the PV it was derived from? Running it through sprintf internally seems like a bit much.

p5pRT commented 13 years ago

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

p5pRT commented 13 years ago

From @samv

On Sun Nov 28 14​:44​:28 2010\, sprout wrote​:

On Sun Jan 07 10​:45​:52 2007\, zefram@​fysh.org wrote​:

$ perl -lwe 'printf "%f\n"\, 1180591620717411303424.0' 1180591620717411172352.000000 $

The value that I put in the source is exactly 2^70.

Perl’s atof implementation (c2988b20) multiplies by ten every time it scans a new digit. This results in loss of precision. What is the best way to tell whether an NV differs from the PV it was derived from? Running it through sprintf internally seems like a bit much.

I performed some investigation of this "bug". No precision is lost by multiplying a small integer in a float by 10. You can see how close it is to being correct from the binary representation​:

perl -le 'print unpack("H*"\, reverse(pack("d"\, 1180591620717411303424.0)))' 444fffffffffffff

This is 1.fffffffffffff * 2^69 and a valid conversion for the input according to at least ieee754-2008 and C99.

It seems true that C99 encourages implementations to perform "correct" rounding\, that is\, returning the binary value which is closest to the input value. However this is just wishful thinking - actual implementation requires MP math; you need to convert the entire input value at whatever precision it is given\, to a binary value of at least 55 or so bits wide. And then apply the rounding rules in binary to find out what the "correct" value is. So there are "halfway points"\, the values above are one binary value and the values below are another. If the input decimal number matches one of these halfway points exactly\, then you have to process some pretty big numbers.

As to how glibc gets it "right". I pulled apart the glibc and indeed some "clever" person actually tried to take the advice in C99 literally; strtod uses GMP internally to try to get this right. Yes\, GMP.

But it still gets it wrong​:

http​://www.exploringbinary.com/incorrectly-rounded-conversions-in-gcc- and-glibc/

This is apparently because the glibc strtod has a "pragmatic" limit of something like 10 extra decimal digits before deciding to ignore the rest. Even that is incorrect\, as the writers in the above article note\, the worst case is over 700 decimal digits of precision that must be calculated before the final answer can be known.

So they just made a string to number conversion not only use MP math and so be very slow\, but they didn't even get the "perfect" behaviour they were seeking. "Excessive cleverness"

Practically speaking\, Perl's implementation is about as good as you could expect from a high performance solution and as far as I can tell confirms to ieee754 rules. If you want the over-engineered version in glibc\, then build your Perl with d_strtod=1 or use a CPAN module which implements the semantics you are after.

I suggest setting this ticket to 'rejected'

p5pRT commented 9 years ago

From zefram@fysh.org

Tom Hukins has run into an interesting case of this bug. He's trying to port Perl to the Beaglebone Black dev board\, which is based on an ARMv6 processor and turns out to have some floating-point oddities. Tom commented "I believe the OS uses software floating point ATM but plans to move over to hardware at some point". The more legitimate of its oddities is that it doesn't support subnormal values. It turns out that this causes the "min double" test in opbasic/arith.t to fail​:

  try $T++\, 2.2250738585072014e-308 != 0.0\, 'min double';

The value shown is not particularly close to the minimum IEEE double-precision value (which is about 4.94e-324\, 2**-1074). It's trying to get the minimum *normal* IEEE double-precision value\, 2**-1022. The decimal value in the test is fractionally above that. Because the Beaglebone system does use IEEE double parameters\, this test is looking at a relevant threshold​: 2**-1022 is not merely its minimum positive normal value\, but actually its minimum positive value overall. In theory the test should pass. (The test is arguably bogus because we don't otherwise require any particular FP range\, but that's a separate issue.)

However\, the duff decimal->float conversion has the effect of breaking the decimal up into a bunch of single-digit subvalues to sum\, all of which in this case are necessarily below the range of normal FP values. On a full IEEE system\, the first few are representable as subnormals\, but eventually they're not representable at all\, and the conversion gets truncated\, quite aside from the rounding error. (But actually\, after the first digit they're not even converted to subnormals\, but instead are erroneously treated as zero; ISTR this being reported as a separate bug somewhere. So overall the test value is converted as if it were only 2.0e-308\, and comes out as a subnormal value that is proportionally quite a lot lower than the proper value.) It's still non-zero\, so the test passes.

On the Beaglebone\, because subnormals can't be handled at all\, all of the single-digit subvalues are computed as zero\, and so the sum is zero. So the test fails\, even though the value the test intends to try is representable distinct from zero.

-zefram

p5pRT commented 6 years ago

From @sisyphus

Le Tue\, 01 Feb 2011 16​:10​:13 -0800\, mugwump a écrit : [snip]

If you want the over-engineered version in glibc\, then build your Perl with d_strtod=1

This is 7 years later\, and even when HAS_STRTOD is defined with the latest devel release(perl-5.27.9) we don't get the value that glibc assigns. And the value we do get is often wrong by more than just one ulp.

I noticed quite some time ago that\, with -Dusequadmath builds of perl\, there are not any known cases of perl assigning a floating point value incorrectly. I'm not saying that all values are assigned correctly on -Dusequadmath builds - merely that I haven't been able to find any incorrect assignments on -Dusequadmath builds.

In contrast\, it's amazingly easy to hit floating point values on double and (extended precision) long double builds of perl that are assigned incorrectly.

So ... what's so special about the way that the quadmath builds assign values ? AFAICT\, the quadmath builds always assign the value that Perl_strtod() returns. But on the 'double' and 'long double' builds\, the (nearly always correct) value returned by Perl_strtod() is ignored in preference to the often incorrect value returned by perl's bodgey atof calculations.

Now\, it seems to me that if Perl_strtod() is good enough for quadmath builds\, then it ought to be good enough for 'double' and 'long double' builds\, too. So\, on Windows and Ubuntu\, I tried a simplistically patched numeric.c that achieved that - and with strikingly improved results for perl-5.27.9.

The patch (attached) ensures that\, if Perl_strtod is defined\, assignment of values for 'double' and 'long double' builds of perl follows the same path as that for '__float128' builds. For me\, Perl_strtod is defined by default on Ubuntu and MS Windows. If Perl_strtod is not defined\, then the patch should have no effect. (Note that perl.h unconditionally defines Perl_strtod to strtoflt128 if USE_QUADMATH is defined.)

The main script I've used for testing is nv2.pl (attached).

On an *unpatched* 5.27.9\, running 'perl nv2.pl 10' has always quickly hit a mis-assigned value (mostly within the first 20 iterations) and died. On the various *patched* builds of 5.27.9 that I've tested\, it has always run the full 1 million iterations without producing any output. Of course\, on the patched builds\, Perl_strtod and C's strtod/strtold/strtoflt128 are the same function\, so it's totally expected that the script will not die when run. What's important\, however\, is that the complete absence of warnings indicates that the assigned values are also correct.

I have managed to find some instances where strtod/strtold does return an incorrect value\, but only on a targeted search of values close to a power of 2. Clearly these are libc bugs - and they are comparatively very rare. I know of only 1 such value on Ubuntu\, and 3 on Windows. As regards quadmath builds\, I haven't found any instances of strtoflt128 assigning incorrectly.

With the patched numeric.c\, all tests still passed on x86 windows for both 'double' and 'long double' builds.(Can't do quadmath builds on Windows.)

Annoyingly\, on the Windows 'long double' build only\, the patch altered the way that the numification of the string '9875'x1000 alters the internals. (This caused one Math​::MPFR test to fail\, and I need to investigate it.)

On Ubuntu\, patched builds produce the following test failure for both the 'double' and the 'long double' build​:

t/porting/args_assert .......................................... # Failed test 2 - PERL_ARGS_ASSERT_GROK_ATOUV is declared but not used at porting/args_assert.t line 62 # Failed test 3 - PERL_ARGS_ASSERT_GROK_BIN is declared but not used at porting/args_assert.t line 62 # Failed test 4 - PERL_ARGS_ASSERT_GROK_HEX is declared but not used at porting/args_assert.t line 62 # Failed test 5 - PERL_ARGS_ASSERT_GROK_INFNAN is declared but not used at porting/args_assert.t line 62 # Failed test 6 - PERL_ARGS_ASSERT_GROK_NUMBER is declared but not used at porting/args_assert.t line 62 # Failed test 7 - PERL_ARGS_ASSERT_GROK_NUMBER_FLAGS is declared but not used at porting/args_assert.t line 62 # Failed test 8 - PERL_ARGS_ASSERT_GROK_NUMERIC_RADIX is declared but not used at porting/args_assert.t line 62 # Failed test 9 - PERL_ARGS_ASSERT_GROK_OCT is declared but not used at porting/args_assert.t line 62 # Failed test 10 - PERL_ARGS_ASSERT_ISINFNANSV is declared but not used at porting/args_assert.t line 62 # Failed test 11 - PERL_ARGS_ASSERT_MY_ATOF is declared but not used at porting/args_assert.t line 62 # Failed test 12 - PERL_ARGS_ASSERT_MY_ATOF2 is declared but not used at porting/args_assert.t line 62 # Failed test 13 - PERL_ARGS_ASSERT_SCAN_BIN is declared but not used at porting/args_assert.t line 62 # Failed test 14 - PERL_ARGS_ASSERT_SCAN_HEX is declared but not used at porting/args_assert.t line 62 # Failed test 15 - PERL_ARGS_ASSERT_SCAN_OCT is declared but not used at porting/args_assert.t line 62 FAILED at test 2

Other than that\, all seems fine.

Is any of this at all useful ? If so\, I'll continue to look for creases\, with an aim to getting them out.

Cheers\, Rob

p5pRT commented 6 years ago

From @sisyphus

nv2.pl

p5pRT commented 6 years ago

From @sisyphus

Hmmm ... I don't see the patch. Resending it.

p5pRT commented 6 years ago

From @sisyphus

--- numeric.c_orig 2018-03-07 19​:31​:23 +1100 +++ numeric.c 2018-03-10 14​:28​:53 +1100 @​@​ -1107\,7 +1107\,7 @​@​   return TRUE; }

-#ifndef USE_QUADMATH +#ifndef Perl_strtod STATIC NV S_mulexp10(NV value\, I32 exponent) { @​@​ -1203\,7 +1203\,7 @​@​   }   return negative ? value / result : value * result; } -#endif /* #ifndef USE_QUADMATH */ +#endif /* #ifndef Perl_strtod */

NV Perl_my_atof(pTHX_ const char* s) @​@​ -1214\,7 +1214\,7 @​@​

  PERL_ARGS_ASSERT_MY_ATOF;

-#ifdef USE_QUADMATH +#ifdef Perl_strtod

  Perl_my_atof2(aTHX_ s\, &x);

@​@​ -1357\,11 +1357\,11 @​@​ {   const char* s = orig;   NV result[3] = {0.0\, 0.0\, 0.0}; -#if defined(USE_PERL_ATOF) || defined(USE_QUADMATH) +#if defined(USE_PERL_ATOF) || defined(Perl_strtod)   const char* send = s + strlen(orig); /* one past the last */   bool negative = 0; #endif -#if defined(USE_PERL_ATOF) && !defined(USE_QUADMATH) +#if defined(USE_PERL_ATOF) && !defined(Perl_strtod)   UV accumulator[2] = {0\,0}; /* before/after dp */   bool seen_digit = 0;   I32 exp_adjust[2] = {0\,0}; @​@​ -1374\,7 +1374\,7 @​@​   I32 sig_digits = 0; /* noof significant digits seen so far */ #endif

-#if defined(USE_PERL_ATOF) || defined(USE_QUADMATH) +#if defined(USE_PERL_ATOF) || defined(Perl_strtod)   PERL_ARGS_ASSERT_MY_ATOF2;

  /* leading whitespace */ @​@​ -1391\,12 +1391\,12 @​@​   } #endif

-#ifdef USE_QUADMATH +#ifdef Perl_strtod   {   char* endp;   if ((endp = S_my_atof_infnan(aTHX_ s\, negative\, send\, value)))   return endp; - result[2] = strtoflt128(s\, &endp); + result[2] = Perl_strtod(s\, &endp);   if (s != endp) {   *value = negative ? -result[2] : result[2];   return endp;

p5pRT commented 6 years ago

From @sisyphus

Le Fri\, 09 Mar 2018 22​:47​:01 -0800\, sisyphus a écrit :

Annoyingly\, on the Windows 'long double' build only\, the patch altered the way that the numification of the string '9875'x1000 alters the internals. (This caused one Math​::MPFR test to fail\, and I need to investigate it.)

Looks to me like a mingw runtime bug in strtold() - which I've reported to​: https://sourceforge.net/p/mingw-w64/bugs/711/

Cheers\, Rob

p5pRT commented 6 years ago

From @sisyphus

On Sun\, 11 Mar 2018 04​:11​:50 -0700\, sisyphus wrote​:

Le Fri\, 09 Mar 2018 22​:47​:01 -0800\, sisyphus a écrit :

Annoyingly\, on the Windows 'long double' build only\, the patch altered the way that the numification of the string '9875'x1000 alters the internals. (This caused one Math​::MPFR test to fail\, and I need to investigate it.)

Looks to me like a mingw runtime bug in strtold() - which I've reported to​: https://sourceforge.net/p/mingw-w64/bugs/711/

The bug turns out to be easily worked around. Attached is the patch I eventually used with perl-5.28.0. On the systems I've tested\, it works flawlessly and I've yet to find a value that gets assigned incorrectly on either "double" builds or extended precision "long double" builds or "__float128" builds of perl. It also does not cause any of the tests (in the perl-5.28.0 test suite) to fail.

The systems I've tested have been Ubuntu-16.04 (gcc-5.4.0 &\, glibc-2.23)\, Debian Wheezy (gcc-4.6.3 & glibc-2.13) and MS Windows (various mingw-w64 ports of gcc-4.7.0 thru to 8.1.0).

I'm therefore unaware of any valid reason that this modified patch should not be incorporated into perl-5.29. If objections exist\, please raise them so that I can attend to them.

The effects of this patch can be nullified by building perl such that HAS_STRTOD or HAS_STRTOLD is not defined. On Linux this is as simple as configuring the perl build with -Ud_strtod or -Ud_strtold.

I've given a little more detail at https://www.perlmonks.org/?node_id=1217588

Cheers\, Rob

p5pRT commented 6 years ago

From @sisyphus

--- numeric.c_orig 2018-07-01 21​:44​:17 +1000 +++ numeric.c 2018-07-01 12​:49​:47 +1000 @​@​ -1119\,7 +1119\,7 @​@​   return TRUE; }

-#ifndef USE_QUADMATH +#ifndef Perl_strtod STATIC NV S_mulexp10(NV value\, I32 exponent) { @​@​ -1215\,7 +1215\,7 @​@​   }   return negative ? value / result : value * result; } -#endif /* #ifndef USE_QUADMATH */ +#endif /* #ifndef Perl_strtod */

NV Perl_my_atof(pTHX_ const char* s) @​@​ -1226\,7 +1226\,7 @​@​

  PERL_ARGS_ASSERT_MY_ATOF;

-#ifdef USE_QUADMATH +#ifdef Perl_strtod

  Perl_my_atof2(aTHX_ s\, &x);

@​@​ -1369\,11 +1369\,11 @​@​ {   const char* s = orig;   NV result[3] = {0.0\, 0.0\, 0.0}; -#if defined(USE_PERL_ATOF) || defined(USE_QUADMATH) +#if defined(USE_PERL_ATOF) || defined(Perl_strtod)   const char* send = s + strlen(orig); /* one past the last */   bool negative = 0; #endif -#if defined(USE_PERL_ATOF) && !defined(USE_QUADMATH) +#if defined(USE_PERL_ATOF) && !defined(Perl_strtod)   UV accumulator[2] = {0\,0}; /* before/after dp */   bool seen_digit = 0;   I32 exp_adjust[2] = {0\,0}; @​@​ -1386\,7 +1386\,7 @​@​   I32 sig_digits = 0; /* noof significant digits seen so far */ #endif

-#if defined(USE_PERL_ATOF) || defined(USE_QUADMATH) +#if defined(USE_PERL_ATOF) || defined(Perl_strtod)   PERL_ARGS_ASSERT_MY_ATOF2;

  /* leading whitespace */ @​@​ -1403\,12 +1403\,27 @​@​   } #endif

-#ifdef USE_QUADMATH +#ifdef Perl_strtod   {   char* endp;   if ((endp = S_my_atof_infnan(aTHX_ s\, negative\, send\, value)))   return endp; - result[2] = strtoflt128(s\, &endp); +# if defined(__MINGW64_VERSION_MAJOR) && defined(USE_LONG_DOUBLE) + + /*********************************************** + We are unable to use strtold because of + https://sourceforge.net/p/mingw-w64/bugs/711/ + & + https://sourceforge.net/p/mingw-w64/bugs/725/ + + but __mingw_strtold is fine. + ***********************************************/ + + result[2] = __mingw_strtold(s\, &endp); + +# else + result[2] = Perl_strtod(s\, &endp);
+# endif   if (s != endp) {   *value = negative ? -result[2] : result[2];   return endp;

p5pRT commented 6 years ago

From @jkeenan

On Sat\, 14 Jul 2018 12​:22​:08 GMT\, sisyphus@​cpan.org wrote​:

The bug turns out to be easily worked around. Attached is the patch I eventually used with perl-5.28.0.

numeric.c has changed since perl-5.28.0\, so your patch did not apply cleanly. There were 3 hunks rejected\, preventing me from creating a smoke-me branch for testing.

Would it be possible to re-draw this patch against perl 5 blead\, preferably creating the patch with 'git format-patch'?

Thank you very much.

Cheers\, Rob

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

p5pRT commented 6 years ago

From @sisyphus

And\, of course\, when I incorporate my patch into the current blead version of numeric.c I find that re/uniprops02.t and re/uniprops03.t both fail a number of tests. All other tests pass. All that my patch does is to make "double" and "long double" builds follow the same path through numeric.c as that taken by the "_float128" builds - and it therefore comes as no surprise that the "__float128" builds (-Dusequadmath)\, built using the official (ie unpatched) numeric.c also fail those same 2 test scripts. (I'll notify p5p of this breakage shortly.)

So ... I think I'll wait for this issue with the quadmath builds to be resolved before investigating further. I'm quietly confident that when that problem with the quadmath builds gets fixed\, then so does the problem with my patch.

The actual patch I used with latest blead (commit 76416d1) is attached.

It would be great if we could have my patch smoked - though perhaps it would be pertinent to wait for the current broken state of blead (wrt quadmath builds) to be fixed.

Thanks for showing some interest\, Jim. It's much appreciated !

Cheers\, Rob

On Sun\, Jul 15\, 2018 at 9​:21 AM\, James E Keenan via RT \< perlbug-followup@​perl.org> wrote​:

On Sat\, 14 Jul 2018 12​:22​:08 GMT\, sisyphus@​cpan.org wrote​:

The bug turns out to be easily worked around. Attached is the patch I eventually used with perl-5.28.0.

numeric.c has changed since perl-5.28.0\, so your patch did not apply cleanly. There were 3 hunks rejected\, preventing me from creating a smoke-me branch for testing.

Would it be possible to re-draw this patch against perl 5 blead\, preferably creating the patch with 'git format-patch'?

Thank you very much.

Cheers\, Rob

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

--- via perlbug​: queue​: perl5 status​: open https://rt-archive.perl.org/perl5/Ticket/Display.html?id=41202

p5pRT commented 6 years ago

From @sisyphus

0001-patched-numeric.c.patch ```diff From 53be06c206a17fea83599e7607ca72d8d2d2d7c3 Mon Sep 17 00:00:00 2001 From: sisyphus Date: Sun, 15 Jul 2018 11:39:59 +1000 Subject: [PATCH] patched numeric.c --- numeric.c | 31 +++++++++++++++++++++++-------- 1 file changed, 23 insertions(+), 8 deletions(-) diff --git a/numeric.c b/numeric.c index 34eb8b3..9f98311 100644 --- a/numeric.c +++ b/numeric.c @@ -1143,7 +1143,7 @@ Perl_grok_atoUV(const char *pv, UV *valptr, const char** endptr) return TRUE; } -#ifndef USE_QUADMATH +#ifndef Perl_strtod STATIC NV S_mulexp10(NV value, I32 exponent) { @@ -1239,7 +1239,7 @@ S_mulexp10(NV value, I32 exponent) } return negative ? value / result : value * result; } -#endif /* #ifndef USE_QUADMATH */ +#endif /* #ifndef Perl_strtod */ NV Perl_my_atof(pTHX_ const char* s) @@ -1250,7 +1250,7 @@ Perl_my_atof(pTHX_ const char* s) PERL_ARGS_ASSERT_MY_ATOF; -#ifdef USE_QUADMATH +#ifdef Perl_strtod my_atof2(s, &x); @@ -1400,13 +1400,13 @@ Perl_my_atof3(pTHX_ const char* orig, NV* value, STRLEN len) { const char* s = orig; NV result[3] = {0.0, 0.0, 0.0}; -#if defined(USE_PERL_ATOF) || defined(USE_QUADMATH) +#if defined(USE_PERL_ATOF) || defined(Perl_strtod) const char* send = s + ((len != 0) ? len : strlen(orig)); /* one past the last */ bool negative = 0; #endif -#if defined(USE_PERL_ATOF) && !defined(USE_QUADMATH) +#if defined(USE_PERL_ATOF) && !defined(Perl_strtod) UV accumulator[2] = {0,0}; /* before/after dp */ bool seen_digit = 0; I32 exp_adjust[2] = {0,0}; @@ -1419,7 +1419,7 @@ Perl_my_atof3(pTHX_ const char* orig, NV* value, STRLEN len) I32 sig_digits = 0; /* noof significant digits seen so far */ #endif -#if defined(USE_PERL_ATOF) || defined(USE_QUADMATH) +#if defined(USE_PERL_ATOF) || defined(Perl_strtod) PERL_ARGS_ASSERT_MY_ATOF3; /* leading whitespace */ @@ -1436,13 +1436,28 @@ Perl_my_atof3(pTHX_ const char* orig, NV* value, STRLEN len) } #endif -#ifdef USE_QUADMATH +#ifdef Perl_strtod { char* endp; if ((endp = S_my_atof_infnan(aTHX_ s, negative, send, value))) return endp; endp = send; - result[2] = strtoflt128(s, &endp); +# if defined(__MINGW64_VERSION_MAJOR) && defined(USE_LONG_DOUBLE) + + /*********************************************** + We are unable to use strtold because of + https://sourceforge.net/p/mingw-w64/bugs/711/ + & + https://sourceforge.net/p/mingw-w64/bugs/725/ + + but __mingw_strtold is fine. + ***********************************************/ + + result[2] = __mingw_strtold(s, &endp); + +# else + result[2] = Perl_strtod(s, &endp); +# endif if (s != endp) { *value = negative ? -result[2] : result[2]; return endp; -- 2.1.4 ```
p5pRT commented 6 years ago

From @sisyphus

And\, of course\, when I incorporate my patch into the current blead version of numeric.c I find that re/uniprops02.t and re/uniprops03.t both fail a number of tests. All other tests pass. All that my patch does is to make "double" and "long double" builds follow the same path through numeric.c as that taken by the "_float128" builds - and it therefore comes as no surprise that the "__float128" builds (-Dusequadmath)\, built using the official (ie unpatched) numeric.c also fail those same 2 test scripts. (I'll notify p5p of this breakage shortly.)

So ... I think I'll wait for this issue with the quadmath builds to be resolved before investigating further. I'm quietly confident that when that problem with the quadmath builds gets fixed\, then so does the problem with my patch.

The actual patch I used with latest blead (commit 76416d1) is attached.

It would be great if we could have my patch smoked - though perhaps it would be pertinent to wait for the current broken state of blead (wrt quadmath builds) to be fixed.

Thanks for showing some interest\, Jim. It's much appreciated !

Cheers\, Rob

On Sun\, Jul 15\, 2018 at 9​:21 AM\, James E Keenan via RT \< perlbug-followup@​perl.org> wrote​:

On Sat\, 14 Jul 2018 12​:22​:08 GMT\, sisyphus@​cpan.org wrote​:

The bug turns out to be easily worked around. Attached is the patch I eventually used with perl-5.28.0.

numeric.c has changed since perl-5.28.0\, so your patch did not apply cleanly. There were 3 hunks rejected\, preventing me from creating a smoke-me branch for testing.

Would it be possible to re-draw this patch against perl 5 blead\, preferably creating the patch with 'git format-patch'?

Thank you very much.

Cheers\, Rob

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

--- via perlbug​: queue​: perl5 status​: open https://rt-archive.perl.org/perl5/Ticket/Display.html?id=41202

p5pRT commented 6 years ago

From @sisyphus

0001-patched-numeric.c.patch ```diff From 53be06c206a17fea83599e7607ca72d8d2d2d7c3 Mon Sep 17 00:00:00 2001 From: sisyphus Date: Sun, 15 Jul 2018 11:39:59 +1000 Subject: [PATCH] patched numeric.c --- numeric.c | 31 +++++++++++++++++++++++-------- 1 file changed, 23 insertions(+), 8 deletions(-) diff --git a/numeric.c b/numeric.c index 34eb8b3..9f98311 100644 --- a/numeric.c +++ b/numeric.c @@ -1143,7 +1143,7 @@ Perl_grok_atoUV(const char *pv, UV *valptr, const char** endptr) return TRUE; } -#ifndef USE_QUADMATH +#ifndef Perl_strtod STATIC NV S_mulexp10(NV value, I32 exponent) { @@ -1239,7 +1239,7 @@ S_mulexp10(NV value, I32 exponent) } return negative ? value / result : value * result; } -#endif /* #ifndef USE_QUADMATH */ +#endif /* #ifndef Perl_strtod */ NV Perl_my_atof(pTHX_ const char* s) @@ -1250,7 +1250,7 @@ Perl_my_atof(pTHX_ const char* s) PERL_ARGS_ASSERT_MY_ATOF; -#ifdef USE_QUADMATH +#ifdef Perl_strtod my_atof2(s, &x); @@ -1400,13 +1400,13 @@ Perl_my_atof3(pTHX_ const char* orig, NV* value, STRLEN len) { const char* s = orig; NV result[3] = {0.0, 0.0, 0.0}; -#if defined(USE_PERL_ATOF) || defined(USE_QUADMATH) +#if defined(USE_PERL_ATOF) || defined(Perl_strtod) const char* send = s + ((len != 0) ? len : strlen(orig)); /* one past the last */ bool negative = 0; #endif -#if defined(USE_PERL_ATOF) && !defined(USE_QUADMATH) +#if defined(USE_PERL_ATOF) && !defined(Perl_strtod) UV accumulator[2] = {0,0}; /* before/after dp */ bool seen_digit = 0; I32 exp_adjust[2] = {0,0}; @@ -1419,7 +1419,7 @@ Perl_my_atof3(pTHX_ const char* orig, NV* value, STRLEN len) I32 sig_digits = 0; /* noof significant digits seen so far */ #endif -#if defined(USE_PERL_ATOF) || defined(USE_QUADMATH) +#if defined(USE_PERL_ATOF) || defined(Perl_strtod) PERL_ARGS_ASSERT_MY_ATOF3; /* leading whitespace */ @@ -1436,13 +1436,28 @@ Perl_my_atof3(pTHX_ const char* orig, NV* value, STRLEN len) } #endif -#ifdef USE_QUADMATH +#ifdef Perl_strtod { char* endp; if ((endp = S_my_atof_infnan(aTHX_ s, negative, send, value))) return endp; endp = send; - result[2] = strtoflt128(s, &endp); +# if defined(__MINGW64_VERSION_MAJOR) && defined(USE_LONG_DOUBLE) + + /*********************************************** + We are unable to use strtold because of + https://sourceforge.net/p/mingw-w64/bugs/711/ + & + https://sourceforge.net/p/mingw-w64/bugs/725/ + + but __mingw_strtold is fine. + ***********************************************/ + + result[2] = __mingw_strtold(s, &endp); + +# else + result[2] = Perl_strtod(s, &endp); +# endif if (s != endp) { *value = negative ? -result[2] : result[2]; return endp; -- 2.1.4 ```
p5pRT commented 6 years ago

From @jkeenan

On Sun\, 15 Jul 2018 04​:29​:42 GMT\, sisyphus359@​gmail.com wrote​:

And\, of course\, when I incorporate my patch into the current blead version of numeric.c I find that re/uniprops02.t and re/uniprops03.t both fail a number of tests. All other tests pass. All that my patch does is to make "double" and "long double" builds follow the same path through numeric.c as that taken by the "_float128" builds - and it therefore comes as no surprise that the "__float128" builds (-Dusequadmath)\, built using the official (ie unpatched) numeric.c also fail those same 2 test scripts. (I'll notify p5p of this breakage shortly.)

On Linux\, I got a lot of failures in 3 files​:

##### re/uniprops02.t (Wstat​: 65280 Tests​: 33506 Failed​: 302)   Failed tests​: 22498-22499\, 22501\, 22503\, 22570\, 26098   26129-26136\, 26202\, 26274\, 26312\, 26346   26348\, 26350\, 26352\, 26418\, 26420\, 26422   26424\, 26439\, 26490\, 26492\, 26494\, 26496   26532\, 26561-26568\, 26582\, 26597\, 26599   26601\, 26603\, 26621\, 26634\, 26636\, 26638-26640   26706\, 26708-26710\, 26712\, 26741\, 26764   26777-26784\, 27272-27273\, 27306\, 27308   27310\, 27312\, 27378\, 27380\, 27382\, 27384   27649-27656\, 27686-27687\, 27689\, 27691   27722\, 27724\, 27726\, 27728\, 27994\, 28035   28066\, 28306\, 28338\, 28379-28380\, 28410   28446\, 28463-28470\, 28482\, 28484\, 28486   28488\, 28962\, 29010\, 29012\, 29014-29016   29028\, 29081\, 29154\, 29226\, 29228\, 29230   29232\, 29268\, 29297-29298\, 29300\, 29302   29304\, 29334\, 29336\, 29370\, 29372\, 29374   29376\, 29411\, 29427\, 29442\, 29444\, 29446   29448\, 29481\, 29483\, 29514\, 29555-29556   29585-29588\, 29590\, 29592\, 29623-29624   29641\, 29658\, 29660\, 29662\, 29664\, 29697   29730\, 29765\, 29771\, 29783\, 29802\, 29804   29806-29808\, 29840\, 29842\, 29844\, 29857   29873-29874\, 29876\, 29878\, 29880\, 29933   29945\, 29981\, 29984-29985\, 30005\, 30017-30018   30020\, 30022\, 30024\, 30042\, 30090\, 30092   30094\, 30096\, 31810\, 31882\, 31935\, 31954   31989\, 31993\, 32025-32026\, 32028\, 32030   32032\, 32084\, 32098\, 32100-32102\, 32104   32170\, 32172\, 32174\, 32176\, 32242\, 32244   32246\, 32248\, 32314\, 32386\, 32458\, 32460-32462   32464\, 32494-32495\, 32497\, 32499\, 32530   32532\, 32534\, 32536\, 32565-32566\, 32601   32674\, 32710\, 32728\, 32746-32748\, 32750   32752\, 32800\, 32818-32820\, 32822\, 32824   33053\, 33060\, 33089-33090\, 33092-33094   33096\, 33161-33166\, 33168\, 33183\, 33203   33233-33240\, 33287\, 33306-33308\, 33310   33312\, 33342\, 33346\, 33378-33380\, 33382   33384\, 33413-33414\, 33416\, 33418\, 33420   33450\, 33452\, 33454\, 33456\, 33486\, 33491   Non-zero exit status​: 255 re/uniprops03.t (Wstat​: 65280 Tests​: 33346 Failed​: 113)   Failed tests​: 196\, 198\, 200\, 202\, 268-269\, 345\, 559\, 1647-1648   2006\, 2080\, 2149\, 2152\, 2224-2225\, 2291   2293-2294\, 3212-3215\, 3358\, 3392\, 3432   3445\, 3464\, 3536\, 3538\, 3540\, 3542\, 3556   3608\, 3680\, 3682\, 3684\, 3686\, 3751-3752   3754\, 3756\, 3758\, 3774\, 3791\, 3824-3826   3828\, 3830\, 3846\, 3860\, 3862-3864\, 3866   3896\, 3898\, 3900\, 3902\, 3936\, 3938\, 3968   3970\, 3972\, 3974\, 4007-4009\, 4040\, 4042   4044\, 4046\, 4282\, 4312\, 4314-4316\, 4318   4348\, 4384\, 4386\, 4388\, 4390\, 4421-4422   4456\, 4458-4462\, 4493\, 4509-4516\, 4528   4530\, 4532-4534\, 4548\, 4564\, 4568\, 4600   4602\, 4604\, 4606   Non-zero exit status​: 255 ../lib/locale.t (Wstat​: 0 Tests​: 682 Failed​: 5)   Failed tests​: 427-428\, 432\, 435\, 449 Files=2661\, Tests=1165509\, 264 wallclock secs (88.37 usr 10.40 sys + 536.50 cusr 29.09 csys = 664.36 CPU) Result​: FAIL makefile​:844​: recipe for target 'test_harness' failed #####

branch​: smoke-me/jkeenan/sisyphus/41202-text-float

So ... I think I'll wait for this issue with the quadmath builds to be resolved before investigating further. I'm quietly confident that when that problem with the quadmath builds gets fixed\, then so does the problem with my patch.

The actual patch I used with latest blead (commit 76416d1) is attached.

It would be great if we could have my patch smoked - though perhaps it would be pertinent to wait for the current broken state of blead (wrt quadmath builds) to be fixed.

Thanks for showing some interest\, Jim. It's much appreciated !

Cheers\, Rob

On Sun\, Jul 15\, 2018 at 9​:21 AM\, James E Keenan via RT \< perlbug-followup@​perl.org> wrote​:

On Sat\, 14 Jul 2018 12​:22​:08 GMT\, sisyphus@​cpan.org wrote​:

The bug turns out to be easily worked around. Attached is the patch I eventually used with perl-5.28.0.

numeric.c has changed since perl-5.28.0\, so your patch did not apply cleanly. There were 3 hunks rejected\, preventing me from creating a smoke-me branch for testing.

Would it be possible to re-draw this patch against perl 5 blead\, preferably creating the patch with 'git format-patch'?

Thank you very much.

Cheers\, Rob

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

--- via perlbug​: queue​: perl5 status​: open https://rt-archive.perl.org/perl5/Ticket/Display.html?id=41202

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

p5pRT commented 6 years ago

From @sisyphus

On Sun\, 15 Jul 2018 06​:14​:33 -0700\, jkeenan wrote​:

On Linux\, I got a lot of failures in 3 files​:

##### re/uniprops02.t (Wstat​: 65280 Tests​: 33506 Failed​: 302) Failed tests​: 22498-22499\, 22501\, 22503\, 22570\, 26098 26129-26136\, 26202\, 26274\, 26312\, 26346 26348\, 26350\, 26352\, 26418\, 26420\, 26422 26424\, 26439\, 26490\, 26492\, 26494\, 26496 26532\, 26561-26568\, 26582\, 26597\, 26599 26601\, 26603\, 26621\, 26634\, 26636\, 26638-26640 26706\, 26708-26710\, 26712\, 26741\, 26764 26777-26784\, 27272-27273\, 27306\, 27308 27310\, 27312\, 27378\, 27380\, 27382\, 27384 27649-27656\, 27686-27687\, 27689\, 27691 27722\, 27724\, 27726\, 27728\, 27994\, 28035 28066\, 28306\, 28338\, 28379-28380\, 28410 28446\, 28463-28470\, 28482\, 28484\, 28486 28488\, 28962\, 29010\, 29012\, 29014-29016 29028\, 29081\, 29154\, 29226\, 29228\, 29230 29232\, 29268\, 29297-29298\, 29300\, 29302 29304\, 29334\, 29336\, 29370\, 29372\, 29374 29376\, 29411\, 29427\, 29442\, 29444\, 29446 29448\, 29481\, 29483\, 29514\, 29555-29556 29585-29588\, 29590\, 29592\, 29623-29624 29641\, 29658\, 29660\, 29662\, 29664\, 29697 29730\, 29765\, 29771\, 29783\, 29802\, 29804 29806-29808\, 29840\, 29842\, 29844\, 29857 29873-29874\, 29876\, 29878\, 29880\, 29933 29945\, 29981\, 29984-29985\, 30005\, 30017-30018 30020\, 30022\, 30024\, 30042\, 30090\, 30092 30094\, 30096\, 31810\, 31882\, 31935\, 31954 31989\, 31993\, 32025-32026\, 32028\, 32030 32032\, 32084\, 32098\, 32100-32102\, 32104 32170\, 32172\, 32174\, 32176\, 32242\, 32244 32246\, 32248\, 32314\, 32386\, 32458\, 32460-32462 32464\, 32494-32495\, 32497\, 32499\, 32530 32532\, 32534\, 32536\, 32565-32566\, 32601 32674\, 32710\, 32728\, 32746-32748\, 32750 32752\, 32800\, 32818-32820\, 32822\, 32824 33053\, 33060\, 33089-33090\, 33092-33094 33096\, 33161-33166\, 33168\, 33183\, 33203 33233-33240\, 33287\, 33306-33308\, 33310 33312\, 33342\, 33346\, 33378-33380\, 33382 33384\, 33413-33414\, 33416\, 33418\, 33420 33450\, 33452\, 33454\, 33456\, 33486\, 33491 Non-zero exit status​: 255 re/uniprops03.t (Wstat​: 65280 Tests​: 33346 Failed​: 113) Failed tests​: 196\, 198\, 200\, 202\, 268-269\, 345\, 559\, 1647-1648 2006\, 2080\, 2149\, 2152\, 2224-2225\, 2291 2293-2294\, 3212-3215\, 3358\, 3392\, 3432 3445\, 3464\, 3536\, 3538\, 3540\, 3542\, 3556 3608\, 3680\, 3682\, 3684\, 3686\, 3751-3752 3754\, 3756\, 3758\, 3774\, 3791\, 3824-3826 3828\, 3830\, 3846\, 3860\, 3862-3864\, 3866 3896\, 3898\, 3900\, 3902\, 3936\, 3938\, 3968 3970\, 3972\, 3974\, 4007-4009\, 4040\, 4042 4044\, 4046\, 4282\, 4312\, 4314-4316\, 4318 4348\, 4384\, 4386\, 4388\, 4390\, 4421-4422 4456\, 4458-4462\, 4493\, 4509-4516\, 4528 4530\, 4532-4534\, 4548\, 4564\, 4568\, 4600 4602\, 4604\, 4606 Non-zero exit status​: 255 ../lib/locale.t (Wstat​: 0 Tests​: 682 Failed​: 5) Failed tests​: 427-428\, 432\, 435\, 449 Files=2661\, Tests=1165509\, 264 wallclock secs (88.37 usr 10.40 sys + 536.50 cusr 29.09 csys = 664.36 CPU) Result​: FAIL makefile​:844​: recipe for target 'test_harness' failed #####

branch​: smoke-me/jkeenan/sisyphus/41202-text-float

That looks similar to what I got\, except I didn't notice any locale.t. Things are much better now\, at blead commit 6d37e916c310482d19ce4cd94848823cad43507a.

The numeric.c patch that I have (against the numeric.c at that commit) is attached as 0002-patched-numeric.c.patch. Let me know if you want a patch against a different stage of numeric.c.

All is working fine for me again - and I hope you see the same improvement.

Note that -Dusequadmath builds of perl should be identical\, irrespective of whether this patch is applied. This is most desirable because quadmath builds already assign floating point values correctly\, straight out of the box. It's only the "double" and "long double" builds that can benefit from this patch - and then only if Perl_strtod is defined.

Having applied the patch\, if you then build perl configured with -Ud_strtod (on a "double" build) or -Ud_strtold (on a "long double" build) then you should effectively disable the patch. (This can be useful for comparison purposes.)

Cheers\, Rob

p5pRT commented 6 years ago

From @sisyphus

0002-patched-numeric.c.patch ```diff From 64d72c34c593bd17e0db25466201a42d358b2866 Mon Sep 17 00:00:00 2001 From: sisyphus Date: Mon, 16 Jul 2018 13:59:19 +1000 Subject: [PATCH] patched numeric.c --- numeric.c | 31 +++++++++++++++++++++++-------- 1 file changed, 23 insertions(+), 8 deletions(-) diff --git a/numeric.c b/numeric.c index b608615..6de333e 100644 --- a/numeric.c +++ b/numeric.c @@ -1143,7 +1143,7 @@ Perl_grok_atoUV(const char *pv, UV *valptr, const char** endptr) return TRUE; } -#ifndef USE_QUADMATH +#ifndef Perl_strtod STATIC NV S_mulexp10(NV value, I32 exponent) { @@ -1239,7 +1239,7 @@ S_mulexp10(NV value, I32 exponent) } return negative ? value / result : value * result; } -#endif /* #ifndef USE_QUADMATH */ +#endif /* #ifndef Perl_strtod */ NV Perl_my_atof(pTHX_ const char* s) @@ -1250,7 +1250,7 @@ Perl_my_atof(pTHX_ const char* s) PERL_ARGS_ASSERT_MY_ATOF; -#ifdef USE_QUADMATH +#ifdef Perl_strtod my_atof2(s, &x); @@ -1400,13 +1400,13 @@ Perl_my_atof3(pTHX_ const char* orig, NV* value, STRLEN len) { const char* s = orig; NV result[3] = {0.0, 0.0, 0.0}; -#if defined(USE_PERL_ATOF) || defined(USE_QUADMATH) +#if defined(USE_PERL_ATOF) || defined(Perl_strtod) const char* send = s + ((len != 0) ? len : strlen(orig)); /* one past the last */ bool negative = 0; #endif -#if defined(USE_PERL_ATOF) && !defined(USE_QUADMATH) +#if defined(USE_PERL_ATOF) && !defined(Perl_strtod) UV accumulator[2] = {0,0}; /* before/after dp */ bool seen_digit = 0; I32 exp_adjust[2] = {0,0}; @@ -1419,7 +1419,7 @@ Perl_my_atof3(pTHX_ const char* orig, NV* value, STRLEN len) I32 sig_digits = 0; /* noof significant digits seen so far */ #endif -#if defined(USE_PERL_ATOF) || defined(USE_QUADMATH) +#if defined(USE_PERL_ATOF) || defined(Perl_strtod) PERL_ARGS_ASSERT_MY_ATOF3; /* leading whitespace */ @@ -1436,7 +1436,7 @@ Perl_my_atof3(pTHX_ const char* orig, NV* value, STRLEN len) } #endif -#ifdef USE_QUADMATH +#ifdef Perl_strtod { char* endp; char* copy = NULL; @@ -1454,7 +1454,22 @@ Perl_my_atof3(pTHX_ const char* orig, NV* value, STRLEN len) s = copy + (s - orig); } - result[2] = strtoflt128(s, &endp); +# if defined(__MINGW64_VERSION_MAJOR) && defined(USE_LONG_DOUBLE) + + /*********************************************** + We are unable to use strtold because of + https://sourceforge.net/p/mingw-w64/bugs/711/ + & + https://sourceforge.net/p/mingw-w64/bugs/725/ + + but __mingw_strtold is fine. + ***********************************************/ + + result[2] = __mingw_strtold(s, &endp); + +# else + result[2] = Perl_strtod(s, &endp); +# endif /* If we created a copy, 'endp' is in terms of that. Convert back to * the original */ -- 2.1.4 ```
p5pRT commented 6 years ago

From @sisyphus

I should probably provide an up-to-date script that checks floating point assignments for correctness. The one I'm now using is attached as atonv.pl. (Some of my earlier attempts did not handle subnormal values correctly.)

The script requires Math-MPFR-4.03\, and that module needs to have been built against mpfr-3.1.6 or later - best to build against the current stable mpfr-4.0.1 if possible. Running the script as\, say\, "perl atonv.pl 324 100000" will select a hundred thousand random values in the range 1e-325 to 10e324 and record any that don't assign correctly.

I'd be interested to hear of any mis-assignments that it catches on perls that define $Config{d_strtod} or $Config{d_strtold} && to which the patch under consideration has been applied.

Cheers\, Rob

p5pRT commented 6 years ago

From @sisyphus

atonv.pl

p5pRT commented 6 years ago

From @sisyphus

The version of numeric.c in the smoke-me/jkeenan/sisyphus/41202-text-float branch looks wrong to me. I don't think it includes the corrections that Karl made in commits d94e901a397f05e1d0030c5cfd535ef97a6808b6 and 6d37e916c310482d19ce4cd94848823cad43507a

Attached is the numeric.c patch against current blead (as at commit b2247a87...)

Could this please be applied to the smoke-me/jkeenan/sisyphus/41202-text-float branch.

Cheers\, Rob

p5pRT commented 6 years ago

From @sisyphus

0003-patched-numeric.c.patch ```diff From 4b3b7f7acf7e121ffc87c844fa2b7c8bad4f2871 Mon Sep 17 00:00:00 2001 From: sisyphus Date: Thu, 19 Jul 2018 21:10:35 +1000 Subject: [PATCH] patched numeric.c --- numeric.c | 31 +++++++++++++++++++++++-------- 1 file changed, 23 insertions(+), 8 deletions(-) diff --git a/numeric.c b/numeric.c index e776f73..40d464a 100644 --- a/numeric.c +++ b/numeric.c @@ -1145,7 +1145,7 @@ Perl_grok_atoUV(const char *pv, UV *valptr, const char** endptr) return TRUE; } -#ifndef USE_QUADMATH +#ifndef Perl_strtod STATIC NV S_mulexp10(NV value, I32 exponent) { @@ -1241,7 +1241,7 @@ S_mulexp10(NV value, I32 exponent) } return negative ? value / result : value * result; } -#endif /* #ifndef USE_QUADMATH */ +#endif /* #ifndef Perl_strtod */ NV Perl_my_atof(pTHX_ const char* s) @@ -1252,7 +1252,7 @@ Perl_my_atof(pTHX_ const char* s) PERL_ARGS_ASSERT_MY_ATOF; -#ifdef USE_QUADMATH +#ifdef Perl_strtod my_atof2(s, &x); @@ -1402,13 +1402,13 @@ Perl_my_atof3(pTHX_ const char* orig, NV* value, STRLEN len) { const char* s = orig; NV result[3] = {0.0, 0.0, 0.0}; -#if defined(USE_PERL_ATOF) || defined(USE_QUADMATH) +#if defined(USE_PERL_ATOF) || defined(Perl_strtod) const char* send = s + ((len != 0) ? len : strlen(orig)); /* one past the last */ bool negative = 0; #endif -#if defined(USE_PERL_ATOF) && !defined(USE_QUADMATH) +#if defined(USE_PERL_ATOF) && !defined(Perl_strtod) UV accumulator[2] = {0,0}; /* before/after dp */ bool seen_digit = 0; I32 exp_adjust[2] = {0,0}; @@ -1421,7 +1421,7 @@ Perl_my_atof3(pTHX_ const char* orig, NV* value, STRLEN len) I32 sig_digits = 0; /* noof significant digits seen so far */ #endif -#if defined(USE_PERL_ATOF) || defined(USE_QUADMATH) +#if defined(USE_PERL_ATOF) || defined(Perl_strtod) PERL_ARGS_ASSERT_MY_ATOF3; /* leading whitespace */ @@ -1438,7 +1438,7 @@ Perl_my_atof3(pTHX_ const char* orig, NV* value, STRLEN len) } #endif -#ifdef USE_QUADMATH +#ifdef Perl_strtod { char* endp; char* copy = NULL; @@ -1456,7 +1456,22 @@ Perl_my_atof3(pTHX_ const char* orig, NV* value, STRLEN len) s = copy + (s - orig); } - result[2] = strtoflt128(s, &endp); +# if defined(__MINGW64_VERSION_MAJOR) && defined(USE_LONG_DOUBLE) + + /*********************************************** + We are unable to use strtold because of + https://sourceforge.net/p/mingw-w64/bugs/711/ + & + https://sourceforge.net/p/mingw-w64/bugs/725/ + + but __mingw_strtold is fine. + ***********************************************/ + + result[2] = __mingw_strtold(s, &endp); + +# else + result[2] = Perl_strtod(s, &endp); +# endif /* If we created a copy, 'endp' is in terms of that. Convert back to * the original */ -- 2.1.4 ```
p5pRT commented 6 years ago

From [Unknown Contact. See original ticket]

The version of numeric.c in the smoke-me/jkeenan/sisyphus/41202-text-float branch looks wrong to me. I don't think it includes the corrections that Karl made in commits d94e901a397f05e1d0030c5cfd535ef97a6808b6 and 6d37e916c310482d19ce4cd94848823cad43507a

Attached is the numeric.c patch against current blead (as at commit b2247a87...)

Could this please be applied to the smoke-me/jkeenan/sisyphus/41202-text-float branch.

Cheers\, Rob

p5pRT commented 6 years ago

From @jkeenan

On Thu\, 19 Jul 2018 11​:48​:24 GMT\, sisyphus@​cpan.org wrote​:

The version of numeric.c in the smoke-me/jkeenan/sisyphus/41202-text- float branch looks wrong to me. I don't think it includes the corrections that Karl made in commits d94e901a397f05e1d0030c5cfd535ef97a6808b6 and 6d37e916c310482d19ce4cd94848823cad43507a

Attached is the numeric.c patch against current blead (as at commit b2247a87...)

Could this please be applied to the smoke-me/jkeenan/sisyphus/41202- text-float branch.

Cheers\, Rob

Replacement branch for smoke-testing​:

smoke-me/jkeenan/sisyphus/41202-2nd-text-float

However\, when I tried that locally\, I encountered previously unseen failures in lib/locale.t​:

##### not ok 427 Verify that a different locale radix works when doing "==" with a constant not ok 428 Verify that a different locale radix works when doing "==" with a scalar ... not ok 432 Verify that "==" with a scalar and an intervening sprintf still works in inner no locale ... ... not ok 435 Verify that after a no-locale block\, a different locale radix still works when doing "==" with a scalar and an intervening sprintf ... not ok 449 Verify atof with locale radix and negative exponent

Failed 5/682 subtests

Test Summary Report


../lib/locale.t (Wstat​: 0 Tests​: 682 Failed​: 5)   Failed tests​: 427-428\, 432\, 435\, 449 Files=1\, Tests=682\, 1 wallclock secs ( 0.06 usr 0.01 sys + 0.65 cusr 0.00 csys = 0.72 CPU) Result​: FAIL #####

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

p5pRT commented 6 years ago

From @khwilliamson

On 07/19/2018 05​:48 AM\, sisyphus@​cpan.org via RT wrote​:

The version of numeric.c in the smoke-me/jkeenan/sisyphus/41202-text-float branch looks wrong to me. I don't think it includes the corrections that Karl made in commits d94e901a397f05e1d0030c5cfd535ef97a6808b6 and 6d37e916c310482d19ce4cd94848823cad43507a

Attached is the numeric.c patch against current blead (as at commit b2247a87...)

Could this please be applied to the smoke-me/jkeenan/sisyphus/41202-text-float branch.

Cheers\, Rob

p5pRT commented 6 years ago

From @khwilliamson

On 07/19/2018 05​:48 AM\, sisyphus@​cpan.org via RT wrote​:

The version of numeric.c in the smoke-me/jkeenan/sisyphus/41202-text-float branch looks wrong to me. I don't think it includes the corrections that Karl made in commits d94e901a397f05e1d0030c5cfd535ef97a6808b6 and 6d37e916c310482d19ce4cd94848823cad43507a

Attached is the numeric.c patch against current blead (as at commit b2247a87...)

Could this please be applied to the smoke-me/jkeenan/sisyphus/41202-text-float branch.

Cheers\, Rob

Again\, I wonder if the Mingw stuff should be moved into perl.h? So that the #ifdef's for that are all in perl.h\, and Perl_strtod behaves consistently everywhere

p5pRT commented 6 years ago

From @sisyphus

On Thu\, 19 Jul 2018 07​:33​:43 -0700\, public@​khwilliamson.com wrote​:

Again\, I wonder if the Mingw stuff should be moved into perl.h? So that the #ifdef's for that are all in perl.h\, and Perl_strtod behaves consistently everywhere

Ok - I'll move the MinGW stuff relating to Perl_strtod into perl.h. As it stands\,Perl_strtod is called only in numerc.c\, but that might not always be the case - and\, besides\, it does make better organizational sense if it's in perl.h.

I'll post again later with the relevant patches.

Cheers\, Rob

p5pRT commented 6 years ago

From @sisyphus

On Thu\, 19 Jul 2018 17​:38​:58 -0700\, sisyphus@​cpan.org wrote​:

I'll post again later with the relevant patches.

AS of current blead (commit cb57a253c69fe8de1944b420abb61efdc241bbad) attached are the relevant patches to perl.h and numeric.c. These patches will remain relevant until perl.h and/or numeric.c are altered.

Cheers\, Rob

p5pRT commented 6 years ago

From @sisyphus

0004-patch-numeric.c.patch ```diff From 7a6a9f8001b9a5ee233bedc580496a9a5da2b3db Mon Sep 17 00:00:00 2001 From: sisyphus Date: Fri, 20 Jul 2018 19:29:32 +1000 Subject: [PATCH 2/2] patch numeric.c and perl.h --- numeric.c | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/numeric.c b/numeric.c index e776f73..2a340d7 100644 --- a/numeric.c +++ b/numeric.c @@ -1145,7 +1145,7 @@ Perl_grok_atoUV(const char *pv, UV *valptr, const char** endptr) return TRUE; } -#ifndef USE_QUADMATH +#ifndef Perl_strtod STATIC NV S_mulexp10(NV value, I32 exponent) { @@ -1241,7 +1241,7 @@ S_mulexp10(NV value, I32 exponent) } return negative ? value / result : value * result; } -#endif /* #ifndef USE_QUADMATH */ +#endif /* #ifndef Perl_strtod */ NV Perl_my_atof(pTHX_ const char* s) @@ -1252,7 +1252,7 @@ Perl_my_atof(pTHX_ const char* s) PERL_ARGS_ASSERT_MY_ATOF; -#ifdef USE_QUADMATH +#ifdef Perl_strtod my_atof2(s, &x); @@ -1402,13 +1402,13 @@ Perl_my_atof3(pTHX_ const char* orig, NV* value, STRLEN len) { const char* s = orig; NV result[3] = {0.0, 0.0, 0.0}; -#if defined(USE_PERL_ATOF) || defined(USE_QUADMATH) +#if defined(USE_PERL_ATOF) || defined(Perl_strtod) const char* send = s + ((len != 0) ? len : strlen(orig)); /* one past the last */ bool negative = 0; #endif -#if defined(USE_PERL_ATOF) && !defined(USE_QUADMATH) +#if defined(USE_PERL_ATOF) && !defined(Perl_strtod) UV accumulator[2] = {0,0}; /* before/after dp */ bool seen_digit = 0; I32 exp_adjust[2] = {0,0}; @@ -1421,7 +1421,7 @@ Perl_my_atof3(pTHX_ const char* orig, NV* value, STRLEN len) I32 sig_digits = 0; /* noof significant digits seen so far */ #endif -#if defined(USE_PERL_ATOF) || defined(USE_QUADMATH) +#if defined(USE_PERL_ATOF) || defined(Perl_strtod) PERL_ARGS_ASSERT_MY_ATOF3; /* leading whitespace */ @@ -1438,7 +1438,7 @@ Perl_my_atof3(pTHX_ const char* orig, NV* value, STRLEN len) } #endif -#ifdef USE_QUADMATH +#ifdef Perl_strtod { char* endp; char* copy = NULL; @@ -1456,7 +1456,7 @@ Perl_my_atof3(pTHX_ const char* orig, NV* value, STRLEN len) s = copy + (s - orig); } - result[2] = strtoflt128(s, &endp); + result[2] = Perl_strtod(s, &endp); /* If we created a copy, 'endp' is in terms of that. Convert back to * the original */ -- 2.1.4 ```
p5pRT commented 6 years ago

From @sisyphus

0004-patch-perl.h.patch ```diff From a732eb381b354d4e567e52790d824d9ba5c1b4a6 Mon Sep 17 00:00:00 2001 From: sisyphus Date: Fri, 20 Jul 2018 19:29:19 +1000 Subject: [PATCH 1/2] patch numeric.c and perl.h --- perl.h | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/perl.h b/perl.h index f4b146d..1fda07c 100644 --- a/perl.h +++ b/perl.h @@ -6477,7 +6477,17 @@ expression, but with an empty argument list, like this: #ifdef USE_QUADMATH # define Perl_strtod(s, e) strtoflt128(s, e) #elif defined(HAS_LONG_DOUBLE) && defined(USE_LONG_DOUBLE) -# if defined(HAS_STRTOLD) +# if defined(__MINGW64_VERSION_MAJOR) && defined(HAS_STRTOLD) + /*********************************************** + We are unable to use strtold because of + https://sourceforge.net/p/mingw-w64/bugs/711/ + & + https://sourceforge.net/p/mingw-w64/bugs/725/ + + but __mingw_strtold is fine. + ***********************************************/ +# define Perl_strtod(s, e) __mingw_strtold(s, e) +# elif defined(HAS_STRTOLD) # define Perl_strtod(s, e) strtold(s, e) # elif defined(HAS_STRTOD) # define Perl_strtod(s, e) (NV)strtod(s, e) /* Unavoidable loss. */ -- 2.1.4 ```
p5pRT commented 6 years ago

From @sisyphus

I believe that the patches (https​://rt.perl.org/Ticket/Attachment/1569082/825260/0004-patch-perl.h.patch and https://rt-archive.perl.org/perl5/Ticket/Attachment/1569082/825259/0004-patch-numeric.c.patch) I provided with my previous post are still applicable to current blead - as there have been no new commits to either perl.h or numeric.c.

What else needs to be done before they can be applied to blead ?

Cheers\, Rob

p5pRT commented 6 years ago

From @khwilliamson

On 07/23/2018 03​:34 AM\, sisyphus@​cpan.org via RT wrote​:

I believe that the patches (https​://rt.perl.org/Ticket/Attachment/1569082/825260/0004-patch-perl.h.patch and https://rt-archive.perl.org/perl5/Ticket/Attachment/1569082/825259/0004-patch-numeric.c.patch) I provided with my previous post are still applicable to current blead - as there have been no new commits to either perl.h or numeric.c.

What else needs to be done before they can be applied to blead ?

Cheers\, Rob

If you look at the previous smoke report\, it's almost all failures.

http​://perl.develop-help.com/?b=smoke-me%2Fjkeenan%2Fsisyphus%2F41202-2nd-text-float

And it's not clear what's causing these. Some failures seem to be occurring to regular blead\, without the patch applied. James Keenan appears to be afk\, so I took your latest patches\, and am doing my own smoke. You can watch it as the results come in

http​://perl.develop-help.com/?b=smoke-me%2Fkhw-sisyphus

--- via perlbug​: queue​: perl5 status​: open https://rt-archive.perl.org/perl5/Ticket/Display.html?id=41202

p5pRT commented 6 years ago

From @khwilliamson

On 07/23/2018 03​:34 AM\, sisyphus@​cpan.org via RT wrote​:

I believe that the patches (https​://rt.perl.org/Ticket/Attachment/1569082/825260/0004-patch-perl.h.patch and https://rt-archive.perl.org/perl5/Ticket/Attachment/1569082/825259/0004-patch-numeric.c.patch) I provided with my previous post are still applicable to current blead - as there have been no new commits to either perl.h or numeric.c.

What else needs to be done before they can be applied to blead ?

Cheers\, Rob

If you look at the previous smoke report\, it's almost all failures.

http​://perl.develop-help.com/?b=smoke-me%2Fjkeenan%2Fsisyphus%2F41202-2nd-text-float

And it's not clear what's causing these. Some failures seem to be occurring to regular blead\, without the patch applied. James Keenan appears to be afk\, so I took your latest patches\, and am doing my own smoke. You can watch it as the results come in

http​://perl.develop-help.com/?b=smoke-me%2Fkhw-sisyphus

--- via perlbug​: queue​: perl5 status​: open https://rt-archive.perl.org/perl5/Ticket/Display.html?id=41202

p5pRT commented 6 years ago

From @jkeenan

On Tue\, 24 Jul 2018 22​:35​:57 GMT\, public@​khwilliamson.com wrote​:

On 07/23/2018 03​:34 AM\, sisyphus@​cpan.org via RT wrote​:

I believe that the patches (https://rt-archive.perl.org/perl5/Ticket/Attachment/1569082/825260/0004-patch- perl.h.patch and https://rt-archive.perl.org/perl5/Ticket/Attachment/1569082/825259/0004-patch- numeric.c.patch) I provided with my previous post are still applicable to current blead - as there have been no new commits to either perl.h or numeric.c.

What else needs to be done before they can be applied to blead ?

Cheers\, Rob

If you look at the previous smoke report\, it's almost all failures.

http​://perl.develop-help.com/?b=smoke-me%2Fjkeenan%2Fsisyphus%2F41202- 2nd-text-float

And it's not clear what's causing these. Some failures seem to be occurring to regular blead\, without the patch applied. James Keenan appears to be afk\, so I took your latest patches\, and am doing my own smoke. You can watch it as the results come in

http​://perl.develop-help.com/?b=smoke-me%2Fkhw-sisyphus

What command-line switches to Configure are needed to smoke-test this branch meaningfully?

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

p5pRT commented 6 years ago

From @sisyphus

On Tue\, 24 Jul 2018 15​:35​:57 -0700\, public@​khwilliamson.com wrote​:

If you look at the previous smoke report\, it's almost all failures.

http​://perl.develop-help.com/?b=smoke-me%2Fjkeenan%2Fsisyphus%2F41202- 2nd-text-float

Some of those failures are on quadmath builds\, and my patches have no effect on quadmath builds. All that my numeric.c changes do is to replace "strtoflt128" with "Perl_strtod" (one and the same thing on quadmath builds) and to replace every occurrence of "USE_QUADMATH" with "Perl_strtod" (but "Perl_strtod" is unconditionally defined in perl.h if "USE_QUADMATH" is defined).

From this\, I deduce that bugs afflicting quadmath builds cannot be attributable to my alterations to perl.h and numeric.c. Those alterations could\, however\, expose "double" and "long double" builds of perl to bugs that previously afflicted only quadmath builds - and I wonder if the lib/locale.t test failures (which I had not seen until a few days ago) might be such a case in point. I can't reproduce those failures on my Ubuntu-16.04 system\, but I see that they occur on both "double" and quadmath builds.

I'll go through the reports there and see if there's anything I should be concerned about.

James Keenan appears to be afk\, so I took your latest patches\, and am doing my own smoke. You can watch it as the results come in

http​://perl.develop-help.com/?b=smoke-me%2Fkhw-sisyphus

Thanks Karl - I'll peruse them with interest. So far there's only a couple of Cygwin reports (or the same report twice) that fail to build because of https://rt-archive.perl.org/perl5/Ticket/Display.html?id=133356

Cheers\, Rob

p5pRT commented 6 years ago

From @sisyphus

On Tue\, 24 Jul 2018 18​:55​:22 -0700\, sisyphus@​cpan.org wrote​:

On Tue\, 24 Jul 2018 15​:35​:57 -0700\, public@​khwilliamson.com wrote​:

You can watch it as the results come in

http​://perl.develop-help.com/?b=smoke-me%2Fkhw-sisyphus

Thanks Karl - I'll peruse them with interest.

The lib/locale.t failures have me stumped for the moment. How do I go about reproducing those failures ? Do I just need to gain access to a NetBSD or FreeBSD system\, or might the locale settings under Carlos' smoker operates be part of the cause ?

I've tried building that smoke-me branch with the same Configure switches as the smoker (-Duse64bitall -Duseithreads -DDEBUGGING) but still can't hit the problem on Ubuntu or Debian.

Cheers\, Rob

Cheers\, Rob

p5pRT commented 6 years ago

From @sisyphus

Just found another couple of spots (one in embed.h\, one in proto.h) where s/USE_QUADMATH/Perl_strtod/ ought to be done. I don't know whether that will account for the lib/locale.t failures\, but I'll test these additional changes on my local machines first - with an aim to testing another smoke-me branch next week if all looks ok here.

Cheers\, Rob

p5pRT commented 6 years ago

From @sisyphus

On Thu\, 26 Jul 2018 06​:37​:30 -0700\, sisyphus@​cpan.org wrote​:

Just found another couple of spots (one in embed.h\, one in proto.h) where s/USE_QUADMATH/Perl_strtod/ ought to be done.

I've since made those changes to proto.h and embed.h. I think they do no more than silence a couple of compiler warnings. I also had to move the Perl_strtod definitions a bit close to the start of perl.h - otherwise embed.h and proto.h are included before Perl_strtod was defined.

The changes cause porting/regen.t to fail tests 37 and 38 because (respectively) proto.h and embed.h have changed - not that there's anything wrong with those changes. How do I make my alterations to embed.h and proto.h without being subjected to such failures ?

And I now also get (on Linux) porting/args_assert.t failures about symbols that were declared but not used. Can I take it as gospel that those symbols were in fact "declared but not used" - or might it simply be that args_assert.t requires some alterations ?

I don't know whether that will account for the lib/locale.t failures\,

It doesn't. Those failures can best be demonstrated by running the following on a perl whose locale radix character is a comma​:

use warnings; use locale; $m = "3.14e+9" + 0; $n = "3\,14e+9" + 0; print "\$m is $m\n"; print "\$n is $n\n"; __END__

On recent perls and current bleadperl (non-quadmath builds) it outputs​: $m is 3140000000 $n is 3140000000

But with my patched perl it outputs $m is 3140000000 $n is 3

The problem appears to be that\, although perls Atof takes the locale radix character into account\, Perl_strtod does not. Quadmath builds of perl (which use Perl_strtod) avoid these failures by skipping the (5) tests that would otherwise fail. The same approach also works fine for my patched "double" and "long double" builds - though I wonder if such a concealment of a change of behaviour might be frowned upon ;-)

Which of the issues just mentioned need to be fixed before my proposed changes to perl.h\, numeric.c\, embed.h\, and proto.h can be applied to perl.

Attached are the latest patches

p5pRT commented 6 years ago

From @sisyphus

0001-embed_h.patch ```diff From e9a380dc315982682b736b91881512ac19d4168f Mon Sep 17 00:00:00 2001 From: sisyphus Date: Wed, 1 Aug 2018 10:57:21 +1000 Subject: [PATCH 1/4] embed_h --- embed.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/embed.h b/embed.h index a2873b9..fa8c6ee 100644 --- a/embed.h +++ b/embed.h @@ -1653,7 +1653,7 @@ #define utf16_textfilter(a,b,c) S_utf16_textfilter(aTHX_ a,b,c) # endif # endif -# if !defined(USE_QUADMATH) +# if !defined(Perl_strtod) # if defined(PERL_IN_NUMERIC_C) #define mulexp10 S_mulexp10 # endif -- 2.1.4 ```
p5pRT commented 6 years ago

From @sisyphus

0002-numeric_c.patch ```diff From dad23802050e0088442e08523664e0429a3298d7 Mon Sep 17 00:00:00 2001 From: sisyphus Date: Wed, 1 Aug 2018 10:58:08 +1000 Subject: [PATCH 2/4] numeric_c --- numeric.c | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/numeric.c b/numeric.c index e776f73..2a340d7 100644 --- a/numeric.c +++ b/numeric.c @@ -1145,7 +1145,7 @@ Perl_grok_atoUV(const char *pv, UV *valptr, const char** endptr) return TRUE; } -#ifndef USE_QUADMATH +#ifndef Perl_strtod STATIC NV S_mulexp10(NV value, I32 exponent) { @@ -1241,7 +1241,7 @@ S_mulexp10(NV value, I32 exponent) } return negative ? value / result : value * result; } -#endif /* #ifndef USE_QUADMATH */ +#endif /* #ifndef Perl_strtod */ NV Perl_my_atof(pTHX_ const char* s) @@ -1252,7 +1252,7 @@ Perl_my_atof(pTHX_ const char* s) PERL_ARGS_ASSERT_MY_ATOF; -#ifdef USE_QUADMATH +#ifdef Perl_strtod my_atof2(s, &x); @@ -1402,13 +1402,13 @@ Perl_my_atof3(pTHX_ const char* orig, NV* value, STRLEN len) { const char* s = orig; NV result[3] = {0.0, 0.0, 0.0}; -#if defined(USE_PERL_ATOF) || defined(USE_QUADMATH) +#if defined(USE_PERL_ATOF) || defined(Perl_strtod) const char* send = s + ((len != 0) ? len : strlen(orig)); /* one past the last */ bool negative = 0; #endif -#if defined(USE_PERL_ATOF) && !defined(USE_QUADMATH) +#if defined(USE_PERL_ATOF) && !defined(Perl_strtod) UV accumulator[2] = {0,0}; /* before/after dp */ bool seen_digit = 0; I32 exp_adjust[2] = {0,0}; @@ -1421,7 +1421,7 @@ Perl_my_atof3(pTHX_ const char* orig, NV* value, STRLEN len) I32 sig_digits = 0; /* noof significant digits seen so far */ #endif -#if defined(USE_PERL_ATOF) || defined(USE_QUADMATH) +#if defined(USE_PERL_ATOF) || defined(Perl_strtod) PERL_ARGS_ASSERT_MY_ATOF3; /* leading whitespace */ @@ -1438,7 +1438,7 @@ Perl_my_atof3(pTHX_ const char* orig, NV* value, STRLEN len) } #endif -#ifdef USE_QUADMATH +#ifdef Perl_strtod { char* endp; char* copy = NULL; @@ -1456,7 +1456,7 @@ Perl_my_atof3(pTHX_ const char* orig, NV* value, STRLEN len) s = copy + (s - orig); } - result[2] = strtoflt128(s, &endp); + result[2] = Perl_strtod(s, &endp); /* If we created a copy, 'endp' is in terms of that. Convert back to * the original */ -- 2.1.4 ```
p5pRT commented 6 years ago

From @sisyphus

0003-perl_h.patch ```diff From 9dc1423d02ea4057c568552b011ef86ff0ba47cd Mon Sep 17 00:00:00 2001 From: sisyphus Date: Wed, 1 Aug 2018 10:58:45 +1000 Subject: [PATCH 3/4] perl_h --- perl.h | 33 ++++++++++++++++++++++----------- 1 file changed, 22 insertions(+), 11 deletions(-) diff --git a/perl.h b/perl.h index f4b146d..16bcf23 100644 --- a/perl.h +++ b/perl.h @@ -1565,6 +1565,28 @@ EXTERN_C char *crypt(const char *, const char *); #define PERL_SNPRINTF_CHECK(len, max, api) STMT_START { if ((max) > 0 && (Size_t)len > (max)) Perl_croak_nocontext("panic: %s buffer overflow", STRINGIFY(api)); } STMT_END #ifdef USE_QUADMATH +# define Perl_strtod(s, e) strtoflt128(s, e) +#elif defined(HAS_LONG_DOUBLE) && defined(USE_LONG_DOUBLE) +# if defined(__MINGW64_VERSION_MAJOR) && defined(HAS_STRTOLD) + /*********************************************** + We are unable to use strtold because of + https://sourceforge.net/p/mingw-w64/bugs/711/ + & + https://sourceforge.net/p/mingw-w64/bugs/725/ + + but __mingw_strtold is fine. + ***********************************************/ +# define Perl_strtod(s, e) __mingw_strtold(s, e) +# elif defined(HAS_STRTOLD) +# define Perl_strtod(s, e) strtold(s, e) +# elif defined(HAS_STRTOD) +# define Perl_strtod(s, e) (NV)strtod(s, e) /* Unavoidable loss. */ +# endif +#elif defined(HAS_STRTOD) +# define Perl_strtod(s, e) strtod(s, e) +#endif + +#ifdef USE_QUADMATH # define my_snprintf Perl_my_snprintf # define PERL_MY_SNPRINTF_GUARDED #elif defined(HAS_SNPRINTF) && defined(HAS_C99_VARIADIC_MACROS) && !(defined(DEBUGGING) && !defined(PERL_USE_GCC_BRACE_GROUPS)) && !defined(PERL_GCC_PEDANTIC) @@ -6474,17 +6496,6 @@ expression, but with an empty argument list, like this: #define Atof my_atof -#ifdef USE_QUADMATH -# define Perl_strtod(s, e) strtoflt128(s, e) -#elif defined(HAS_LONG_DOUBLE) && defined(USE_LONG_DOUBLE) -# if defined(HAS_STRTOLD) -# define Perl_strtod(s, e) strtold(s, e) -# elif defined(HAS_STRTOD) -# define Perl_strtod(s, e) (NV)strtod(s, e) /* Unavoidable loss. */ -# endif -#elif defined(HAS_STRTOD) -# define Perl_strtod(s, e) strtod(s, e) -#endif #if !defined(Strtol) && defined(USE_64_BIT_INT) && defined(IV_IS_QUAD) && \ (QUADKIND == QUAD_IS_LONG_LONG || QUADKIND == QUAD_IS___INT64) -- 2.1.4 ```
p5pRT commented 6 years ago

From @sisyphus

proto.h

p5pRT commented 6 years ago

From @sisyphus

Attached are the latest patches

Ignore the proto.h attachment. Attached (with any luck) will be the patch to proto.h.

p5pRT commented 6 years ago

From @sisyphus

0004-proto_h.patch ```diff From 53de570889de38a9cc7e915f195ab2bd4c2de97a Mon Sep 17 00:00:00 2001 From: sisyphus Date: Wed, 1 Aug 2018 10:59:39 +1000 Subject: [PATCH 4/4] proto.h --- proto.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/proto.h b/proto.h index b94a47d..2b4c647 100644 --- a/proto.h +++ b/proto.h @@ -4327,7 +4327,7 @@ STATIC void S_validate_suid(pTHX_ PerlIO *rsfp); assert(rsfp) # endif #endif -#if !defined(USE_QUADMATH) +#if !defined(Perl_strtod) # if defined(PERL_IN_NUMERIC_C) STATIC NV S_mulexp10(NV value, I32 exponent); # endif -- 2.1.4 ```
p5pRT commented 6 years ago

From @khwilliamson

On 07/31/2018 07​:07 PM\, sisyphus@​cpan.org via RT wrote​:

On Thu\, 26 Jul 2018 06​:37​:30 -0700\, sisyphus@​cpan.org wrote​:

Just found another couple of spots (one in embed.h\, one in proto.h) where s/USE_QUADMATH/Perl_strtod/ ought to be done.

I've since made those changes to proto.h and embed.h. I think they do no more than silence a couple of compiler warnings. I also had to move the Perl_strtod definitions a bit close to the start of perl.h - otherwise embed.h and proto.h are included before Perl_strtod was defined.

The changes cause porting/regen.t to fail tests 37 and 38 because (respectively) proto.h and embed.h have changed - not that there's anything wrong with those changes. How do I make my alterations to embed.h and proto.h without being subjected to such failures ?

embed.fnc should be patched with the necessary #ifdef's. Then regenerating things will cause proto.h and embed.h to fall into line.

And I now also get (on Linux) porting/args_assert.t failures about symbols that were declared but not used. Can I take it as gospel that those symbols were in fact "declared but not used" - or might it simply be that args_assert.t requires some alterations ?

I think it's pretty close to gospel.

I don't know whether that will account for the lib/locale.t failures\,

It doesn't. Those failures can best be demonstrated by running the following on a perl whose locale radix character is a comma​:

use warnings; use locale; $m = "3.14e+9" + 0; $n = "3\,14e+9" + 0; print "\$m is $m\n"; print "\$n is $n\n"; __END__

On recent perls and current bleadperl (non-quadmath builds) it outputs​: $m is 3140000000 $n is 3140000000

But with my patched perl it outputs $m is 3140000000 $n is 3

The problem appears to be that\, although perls Atof takes the locale radix character into account\, Perl_strtod does not. Quadmath builds of perl (which use Perl_strtod) avoid these failures by skipping the (5) tests that would otherwise fail. The same approach also works fine for my patched "double" and "long double" builds - though I wonder if such a concealment of a change of behaviour might be frowned upon ;-)

I'm disappointed to learn that these builds were added with such concealment.

Which of the issues just mentioned need to be fixed before my proposed changes to perl.h\, numeric.c\, embed.h\, and proto.h can be applied to perl.

Attached are the latest patches

I have pushed a branch that I think may add locale handling to quadmath.   I'm having trouble getting quadmath to compile on my machine. Please try it to see how it works. You probably can just add your patches on top of it.

https://perl5.git.perl.org/perl.git/shortlog/refs/heads/smoke-me/khw-locale.

--- via perlbug​: queue​: perl5 status​: open https://rt-archive.perl.org/perl5/Ticket/Display.html?id=41202

p5pRT commented 6 years ago

From @khwilliamson

On 07/31/2018 07​:07 PM\, sisyphus@​cpan.org via RT wrote​:

On Thu\, 26 Jul 2018 06​:37​:30 -0700\, sisyphus@​cpan.org wrote​:

Just found another couple of spots (one in embed.h\, one in proto.h) where s/USE_QUADMATH/Perl_strtod/ ought to be done.

I've since made those changes to proto.h and embed.h. I think they do no more than silence a couple of compiler warnings. I also had to move the Perl_strtod definitions a bit close to the start of perl.h - otherwise embed.h and proto.h are included before Perl_strtod was defined.

The changes cause porting/regen.t to fail tests 37 and 38 because (respectively) proto.h and embed.h have changed - not that there's anything wrong with those changes. How do I make my alterations to embed.h and proto.h without being subjected to such failures ?

embed.fnc should be patched with the necessary #ifdef's. Then regenerating things will cause proto.h and embed.h to fall into line.

And I now also get (on Linux) porting/args_assert.t failures about symbols that were declared but not used. Can I take it as gospel that those symbols were in fact "declared but not used" - or might it simply be that args_assert.t requires some alterations ?

I think it's pretty close to gospel.

I don't know whether that will account for the lib/locale.t failures\,

It doesn't. Those failures can best be demonstrated by running the following on a perl whose locale radix character is a comma​:

use warnings; use locale; $m = "3.14e+9" + 0; $n = "3\,14e+9" + 0; print "\$m is $m\n"; print "\$n is $n\n"; __END__

On recent perls and current bleadperl (non-quadmath builds) it outputs​: $m is 3140000000 $n is 3140000000

But with my patched perl it outputs $m is 3140000000 $n is 3

The problem appears to be that\, although perls Atof takes the locale radix character into account\, Perl_strtod does not. Quadmath builds of perl (which use Perl_strtod) avoid these failures by skipping the (5) tests that would otherwise fail. The same approach also works fine for my patched "double" and "long double" builds - though I wonder if such a concealment of a change of behaviour might be frowned upon ;-)

I'm disappointed to learn that these builds were added with such concealment.

Which of the issues just mentioned need to be fixed before my proposed changes to perl.h\, numeric.c\, embed.h\, and proto.h can be applied to perl.

Attached are the latest patches

I have pushed a branch that I think may add locale handling to quadmath.   I'm having trouble getting quadmath to compile on my machine. Please try it to see how it works. You probably can just add your patches on top of it.

https://perl5.git.perl.org/perl.git/shortlog/refs/heads/smoke-me/khw-locale.

--- via perlbug​: queue​: perl5 status​: open https://rt-archive.perl.org/perl5/Ticket/Display.html?id=41202

p5pRT commented 6 years ago

From @sisyphus

On Tue\, 31 Jul 2018 20​:47​:53 -0700\, public@​khwilliamson.com wrote​:

I have pushed a branch that I think may add locale handling to quadmath.

Yes\, that's looking good here as a quadmath build (built with "-Duse64bitall -Dusequadmath"). There's no sign of the locale problems that I see with blead. Your khw-locale branch hadn't made any changes to lib/locale.t - so I replaced all 5 occurrences of "if($Config{usequadmath})" with "if(0)" and it still passed. And "3\,14e+9"+0 is now being evaluated correctly.

The only test failure on the quadmath build was lib/File/Copy.t which is a separate issue (https://rt-archive.perl.org/perl5/Ticket/Display.html?id=133377).

I then stuck my earlier perl.h and numeric.c changes on top of that branch - didn't bother with the proto.h and embed.h patches as that's only going to confuse things at the moment. I then rebuilt with "-Duse64bitall" (nvtype is double). All tests passed on this Ubuntu system with the (previously) troublesome locale setting !!

I then re-built the quadmath build\, in case my additional changes might've upset it. (Thankfully\, they had not.)

I also checked a uselongdouble 64-bit build on Windows\, too\, and I'll check other builds as time permits.

Everything looks good so far.

Attached are the patches I applied to your smoke-me/khw-locale branch. (The patch for lib/locale.t disables the quadmath special casing - but I ought to re-work it before it's ready for blead.)

p5pRT commented 6 years ago

From @sisyphus

On Wed\, 01 Aug 2018 07​:04​:30 -0700\, sisyphus@​cpan.org wrote​:

Attached are the patches I applied to your smoke-me/khw-locale branch.

Jesus ... where the hell did they go :-(