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
op/pack.t failures with PPC long double (double double) builds #14812
Comments
From @sisyphusHi, The op/pack.t test script fails with both perl-5.23.1 and perl-5.22.0. See the attached pack.fail for the failing tests and their diagnostics. Perl -V is: #################################################### Platform: Characteristics of this binary (from libperl): Cheers, |
From @jkeenanOn Thu Jul 23 06:47:02 2015, sisyphus wrote:
1. In the section of INSTALL discussing "64 bit support", I read: ##### So, to eliminate one possible source of noise, could you re-compile without explicitly saying either '-Duse64bitint' or '-Duse64bitall'? When you do so, do you get the same results? Along the same lines, when you build an unthreaded perl, do you get the same failures? 2. Do you know where your architecture stands with respect to the concerns raised in the "Long doubles" section of INSTALL? ##### Note that the exact format and range of long doubles varies: -- |
The RT System itself - Status changed from 'new' to 'open' |
From @jhiOn Sunday-201508-02 8:19, James E Keenan via RT wrote:
Darn. That's wrong and that's my fault (257c99f). That was for As for Sisyphus' quandary: the "double double" is "long double", |
From @sisyphus-----Original Message-----
As pointed out on wikipedia ( https://en.wikipedia.org/wiki/Long_double ), Jim, I don't have any reason to believe that the removal of -Duse64bitint The same op/pack.t failures occurred with 5.20.0 - no regressions, as far as Cheers, |
From @sisyphus-----Original Message-----
Yes.
Yes. Cheers, |
From @jhiLooking at the pattern of failures, it is something like... for $x (qw(c s s> s< i i< i> l l> l< s! s!> s!< i! i!> i!< l! l!> l!< n! v! N! V! q Q j j> j<)) { So it's the checksum computing pack/unpack, and for 65 bits. (The previous checksum sizes, including 63 and 64, have passed the tests.) |
From @jhiMy suspicion hovers somewhere around line 1760 in pp_pack.c, this spot: ... Sisyphys, could you add PerlIO_printf(Perl_debug_log, "cdouble = %"NVgf"\n", cdouble); and the same for anv, whenever they have been updated? In Perl terms, e.g. ./perl -wle 'print unpack("%65s",pack("s*",-32768,-1,0,1,32764))' The 3...e+19 is the 36893488147419103231 the pack.t was expecting, but got zero. Of especial suspicion is the modf call in there. Does that work right for these double-doubles? |
From [Unknown Contact. See original ticket]My suspicion hovers somewhere around line 1760 in pp_pack.c, this spot: ... Sisyphys, could you add PerlIO_printf(Perl_debug_log, "cdouble = %"NVgf"\n", cdouble); and the same for anv, whenever they have been updated? In Perl terms, e.g. ./perl -wle 'print unpack("%65s",pack("s*",-32768,-1,0,1,32764))' The 3...e+19 is the 36893488147419103231 the pack.t was expecting, but got zero. Of especial suspicion is the modf call in there. Does that work right for these double-doubles? |
From @sisyphus-----Original Message-----
I might not get to that until the weekend.
With both perl-5.22.0 and perl-5.23.1 I get: ./perl -wle 'print unpack("%65s",pack("s*",-32768,-1,0,1,32764))'
I note that the value I got is much closer to the value you got than to the The test 694 failure diagnostics tell me: How do I then replicate that failure once perl has been installed ? $ perl -wle 'print unpack("%65s",pack("s*",-32768, -1, 0, 1, 32767))' $ perl -wle 'print unpack("%65s",pack("s>",-32768, -1, 0, 1, 32767))' $ perl -wle 'print unpack("%65s",pack("s",-32768, -1, 0, 1, 32767))' Just what is the actual command that's returning the '0' ?
Perl_modf() seems ok to me. 1.41421356237309504880168872421 4.123105625617660549821409855974 Cheers, |
From @sisyphus |
From @jhi
Umm. Now this is strange (on a x86_64 system): perl -wle 'print unpack("%65s*",pack("s*",-32768,-1,0,1,32767))' (as opposed to '%64': perl -wle 'print unpack("%64s*",pack("s*",-32768,-1,0,1,32767))' So it seems the 0 is the really expected value with %65? If this is the case, then pack.t 711 if ($calc_sum == $calc_sum - 1 && $calc_sum == $max_p1) { Sisyphus, could you debug some in pack.t? |
From [Unknown Contact. See original ticket]
Umm. Now this is strange (on a x86_64 system): perl -wle 'print unpack("%65s*",pack("s*",-32768,-1,0,1,32767))' (as opposed to '%64': perl -wle 'print unpack("%64s*",pack("s*",-32768,-1,0,1,32767))' So it seems the 0 is the really expected value with %65? If this is the case, then pack.t 711 if ($calc_sum == $calc_sum - 1 && $calc_sum == $max_p1) { Sisyphus, could you debug some in pack.t? |
From @sisyphus-----Original Message-----
Yes, I think that line (line 611) makes assumptions that just don't hold for I've verified that the condition ($calc_sum == $calc_sum - 1 && $calc_sum == Judging by the associated comments, the underlying assumption of line 611 I notice that the failing tests have invariably given 0, with an expected But when I run the actual failing test, I invariably get a value that is # For list (-2147483648, -1, 0, 1, 2147483647) (total -1) packed with j< But when I run that code from the command line I get something entirely 65-bit binary representation of 36893488145271619584 is: (That's 34 set bits followed by 31 unset bits.) Now .... it's very easy to get all of the pack.t tests to pass. (($calc_sum == $calc_sum - 1 && $calc_sum == $max_p1) || $calc_sum >= 2 ** I think that is (maybe) a fair enough rewrite of the condition - assuming But, although pack.t then passes all tests, determining that the previously I deduce 2 things from this: I'll post this off as is - though I hope to make further progress today. Questions: Cheers, |
From @sisyphus-----Original Message-----
As suggested, I replaced that section with: if (checksum) { anv = (NV) (1 << (checksum & 15)); I then get: $ ./perl -I../lib -wle 'print By my calculations, the final value for anv (3.68935e+19) is, in fact, Cheers, |
From @sisyphus-----Original Message-----
Ooops ... the actual "gave" value is determined a few lines higher up in my $sum = eval {unpack "%$_$format*", pack "$format*", @_}; So I was running the wrong command. The command I should have run is: $ ../perl -I../lib -wle 'print Which does, in fact, agree with the "gave" value !! We just need to alter the pack.t code so that the calculation of the I was thinking again of my earlier suggestion of changing: if ($calc_sum == $calc_sum - 1 && $calc_sum == $max_p1) { BUT ... one problem with that solution is that, if pack.t were ever to test I'll see if I can come up with a patch to pack.t such that the tests would Incidentally, the lowest value I've found for $x such that $x == $x-1 is Cheers, |
From @sisyphus-----Original Message-----
Disregard all that I've written previously. At values sufficiently close to 1 and -1 (both above and below), modfl() and The attached patch to pp_pack.c works around the problem in so far as it First question is, do we bother about this ? With this patch in place, I still get (as before): ../perl -I../lib -le 'print But if I go so far as to change line 570 of pack.t from: my $sum = eval {unpack "%$_$format*", pack "$format*", @_}; I get lots of failures and a hang. (Does it make sense to run such tests ?) Cheers, |
From @sisyphuspp_pack.c.patch--- pp_pack.c 2015-08-09 17:37:55 +1000
+++ pp_pack.c_new 2015-08-12 01:36:11 +1000
@@ -1767,7 +1767,17 @@
}
while (cdouble < 0.0)
cdouble += anv;
- cdouble = Perl_modf(cdouble / anv, &trouble) * anv;
+ cdouble = Perl_modf(cdouble / anv, &trouble);
+
+ /* workaround powerpc doubledouble modfl bug */
+ /* close to 1 and -1 cdouble is 0, and trouble is cdouble / anv */
+ if(cdouble == Perl_ceil(cdouble) && trouble != Perl_ceil(trouble)) {
+ if(trouble > 1) trouble -= 1; /* not needed for op/pack.t to pass */
+ if(trouble < -1) trouble += 1; /* not needed for op/pack.t to pass */
+ cdouble = trouble;
+ } /* end of workaround */
+
+ cdouble *= anv;
sv = newSVnv(cdouble);
}
else {
|
From @jhi(the discussion on this went off-list, but eventually Sisyphus came up with a nice fix, I'm pasting his final analysis here in full) --- paste --- There's a number of things wrong with your patch. Firstly, Perl_modf() can get it wrong for values greater than 1. The problem I know of (essentially) only one problem value - namely plus or minus Case A Case B Case C (-ve version of Case A) Case D (-ve version of Case B) Now, as regards, op/pack.t, the only case we actually hit is that of Case A. Is it possible to strike Case C or Case D in the calculation of checksums ? Your patch looks at the value of cdouble following the running of: cdouble = Perl_modf(cdouble / anv, &trouble) * anv; but we need to examine what the Perl_modf() call did, not what was done by Also, you're seeking to detect cdouble != Perl_ceil(cdouble), but that just I like the inclusion of the LONGDOUBLE_DOUBLEDOUBLE condition, so I've added The attached pp_pack.c_patch is working fine for me - though there are + if(cdouble > 1.0L) cdouble -= 1.0L; I've put them in there so that the right thing might be done if Cases B or D In my patch, it occurs to me that: cdouble *= anv; might better be replaced with: sv = newSVnv(cdouble * anv); but that would leave cdouble hanging with a different value. (I don't think I re-ran 'make test' in its entirety after building with the attached patch, |
From @jhiFix now in as http://perl5.git.perl.org/perl.git/commit/09b94b1f0efd8c107548a6fefcd471e9b06c2cdf |
@jhi - Status changed from 'open' to 'resolved' |
Migrated from rt.perl.org#125669 (status was 'resolved')
Searchable as RT125669$
The text was updated successfully, but these errors were encountered: