Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

op/pack.t failures with PPC long double (double double) builds #14812

Closed
p5pRT opened this issue Jul 23, 2015 · 23 comments
Closed

op/pack.t failures with PPC long double (double double) builds #14812

p5pRT opened this issue Jul 23, 2015 · 23 comments

Comments

@p5pRT
Copy link

p5pRT commented Jul 23, 2015

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

Searchable as RT125669$

@p5pRT
Copy link
Author

p5pRT commented Jul 23, 2015

From @sisyphus

Hi,

The op/pack.t test script fails with both perl-5.23.1 and perl-5.22.0.
(Details given here relate to perl-5.23.1, but the failures with 5.22.0 are
identical.)

See the attached pack.fail for the failing tests and their diagnostics.

Perl -V is​:

####################################################
Summary of my perl5 (revision 5 version 23 subversion 1) configuration​:

  Platform​:
  osname=linux, osvers=3.2.0-4-powerpc64,
archname=ppc64-linux-thread-multi-ld
  uname='linux debian-sis 3.2.0-4-powerpc64 #1 smp debian 3.2.68-1+deb7u2
ppc64 gnulinux '
  config_args='-des -Duse64bitint -Dusethreads -Duselongdouble -Duse64bitall
-Dcc=gcc -m64 -Dprefix=/home/sisyphus-sis/bleadperl -Dusedevel'
  hint=recommended, useposix=true, d_sigaction=define
  useithreads=define, usemultiplicity=define
  use64bitint=define, use64bitall=define, uselongdouble=define
  usemymalloc=n, bincompat5005=undef
  Compiler​:
  cc='gcc -m64', ccflags
='-D_REENTRANT -D_GNU_SOURCE -fwrapv -fno-strict-aliasing -pipe -fstack-protector
-I/usr/local/include -D_LARGEFILE_SOURCE -D_FILE_OFFSET_BITS=64 -D_FORTIFY_SOURCE=2',
  optimize='-O1',
  cppflags='-D_REENTRANT -D_GNU_SOURCE -fwrapv -fno-strict-aliasing -pipe
-fstack-protector -I/usr/local/include'
  ccversion='', gccversion='4.6.3', gccosandvers=''
  intsize=4, longsize=8, ptrsize=8, doublesize=8, byteorder=87654321,
doublekind=4
  d_longlong=define, longlongsize=8, d_longdbl=define, longdblsize=16,
longdblkind=6
  ivtype='long', ivsize=8, nvtype='long double', nvsize=16, Off_t='off_t',
lseeksize=8
  alignbytes=16, prototype=define
  Linker and Libraries​:
  ld='gcc -m64', ldflags =' -fstack-protector -L/usr/local/lib'
  libpth=/usr/local/lib /usr/lib/gcc/powerpc-linux-gnu/4.6/include-fixed
/usr/lib /lib/powerpc-linux-gnu /lib/../lib /usr/lib/powerpc-linux-gnu
/usr/lib/../lib /lib /lib64 /usr/lib64
  libs=-lpthread -lnsl -ldl -lm -lcrypt -lutil -lc
  perllibs=-lpthread -lnsl -ldl -lm -lcrypt -lutil -lc
  libc=libc-2.13.so, so=so, useshrplib=false, libperl=libperl.a
  gnulibc_version='2.13'
  Dynamic Linking​:
  dlsrc=dl_dlopen.xs, dlext=so, d_dlsymun=undef, ccdlflags='-Wl,-E'
  cccdlflags='-fPIC',
lddlflags='-shared -O1 -L/usr/local/lib -fstack-protector'

Characteristics of this binary (from libperl)​:
  Compile-time options​: HAS_TIMES MULTIPLICITY PERLIO_LAYERS
  PERL_COPY_ON_WRITE PERL_DONT_CREATE_GVSV
  PERL_HASH_FUNC_ONE_AT_A_TIME_HARD
  PERL_IMPLICIT_CONTEXT PERL_MALLOC_WRAP
  PERL_PRESERVE_IVUV PERL_USE_DEVEL USE_64_BIT_ALL
  USE_64_BIT_INT USE_ITHREADS USE_LARGE_FILES
  USE_LOCALE USE_LOCALE_COLLATE USE_LOCALE_CTYPE
  USE_LOCALE_NUMERIC USE_LOCALE_TIME USE_LONG_DOUBLE
  USE_PERLIO USE_PERL_ATOF USE_REENTRANT_API
  Built under linux
  Compiled at Jul 23 2015 21​:10​:35
  @​INC​:
  ../lib
  /home/sisyphus-sis/bleadperl/lib/site_perl/5.23.1/ppc64-linux-thread-multi-ld
  /home/sisyphus-sis/bleadperl/lib/site_perl/5.23.1
  /home/sisyphus-sis/bleadperl/lib/5.23.1/ppc64-linux-thread-multi-ld
  /home/sisyphus-sis/bleadperl/lib/5.23.1
  /home/sisyphus-sis/bleadperl/lib/site_perl
  .
####################################################

Cheers,
Rob

@p5pRT
Copy link
Author

p5pRT commented Jul 23, 2015

From @sisyphus

pack.fail

@p5pRT
Copy link
Author

p5pRT commented Aug 2, 2015

From @jkeenan

On Thu Jul 23 06​:47​:02 2015, sisyphus wrote​:

Hi,

The op/pack.t test script fails with both perl-5.23.1 and perl-5.22.0.
(Details given here relate to perl-5.23.1, but the failures with
5.22.0 are
identical.)

See the attached pack.fail for the failing tests and their
diagnostics.

Perl -V is​:

####################################################
Summary of my perl5 (revision 5 version 23 subversion 1)
configuration​:

Platform​:
osname=linux, osvers=3.2.0-4-powerpc64,
archname=ppc64-linux-thread-multi-ld
uname='linux debian-sis 3.2.0-4-powerpc64 #1 smp debian 3.2.68-
1+deb7u2
ppc64 gnulinux '
config_args='-des -Duse64bitint -Dusethreads -Duselongdouble
-Duse64bitall
-Dcc=gcc -m64 -Dprefix=/home/sisyphus-sis/bleadperl -Dusedevel'

1. In the section of INSTALL discussing "64 bit support", I read​:

#####
Natively 64-bit systems need neither -Duse64bitint nor -Duse64bitall.
On these systems, it might be the default compilation mode ....
#####

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?

#####
In some systems you may be able to use long doubles to enhance the
range and precision of your double precision floating point numbers
(that is, Perl's numbers). Use Configure -Duselongdouble to enable
this support (if it is available).

Note that the exact format and range of long doubles varies​:
the most common is the x86 80-bit (64 bits of mantissa) format,
but there are others, with different mantissa and exponent ranges.
In fact, the type may not be called "long double" at C level, and
therefore the C<uselongdouble> means "using floating point larger
than double".
#####

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

@p5pRT
Copy link
Author

p5pRT commented Aug 2, 2015

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

@p5pRT
Copy link
Author

p5pRT commented Aug 2, 2015

From @jhi

On Sunday-201508-02 8​:19, James E Keenan via RT wrote​:

In fact, the type may not be called "long double" at C level, and
therefore the C<uselongdouble> means "using floating point larger
than double

Darn. That's wrong and that's my fault (257c99f). That was for
a while a true statement, while I was working on the quadmath, but
it is no more true. (Though, one could argue that for allowing
more leeway in future one should leave that fibbing in.) I tried
undoing the false earlier (568793b) but missed the above spot.

As for Sisyphus' quandary​: the "double double" is "long double",
the "double double" it's just one of the many possible implementations
of "long double".

@p5pRT
Copy link
Author

p5pRT commented Aug 2, 2015

From @jhi

On Sunday-201508-02 8​:38, Jarkko Hietaniemi wrote​:

more leeway in future one should leave that fibbing in.) I tried
undoing the false earlier (568793b) but missed the above spot.

Darn, I meant b7ce25d. And now I made the same misquote in 9380394.

@p5pRT
Copy link
Author

p5pRT commented Aug 4, 2015

From @sisyphus

-----Original Message-----
From​: Jarkko Hietaniemi
Sent​: Sunday, August 02, 2015 10​:38 PM
To​: perlbug-followup@​perl.org ; sisyphus1@​optusnet.com.au
Subject​: Re​: [perl #125669] op/pack.t failures with PPC long double (double
double) builds

As for Sisyphus' quandary​: the "double double" is "long double", the
"double double" it's just one of the many possible implementations of
"long double".

As pointed out on wikipedia ( https://en.wikipedia.org/wiki/Long_double ),
this format doesn't conform to " IEEE floating-point standard" (
https://en.wikipedia.org/wiki/IEEE_floating-point_standard ).
But it is the C "long double", as implemented by Debian wheezy's gcc-4.6.3
for this OS - which does not mean that perl invariably assigns the exact
same value as C.
There are bugs in libc and perl (and perhaps even the C compiler itself)
that create discrepancies in the assignment of certain values.

Jim, I don't have any reason to believe that the removal of -Duse64bitint
and/or -Duse64bitall and/or -Dusethreads will make a difference, but I
haven't yet verified that (to the extent of specifically testing it).
WRT -Duesthreads, it made no difference to any test results for 5.20.0.

The same op/pack.t failures occurred with 5.20.0 - no regressions, as far as
I've noticed !!

Cheers,
Rob

@p5pRT
Copy link
Author

p5pRT commented Aug 6, 2015

From @sisyphus

-----Original Message-----
From​: James E Keenan via RT
Sent​: Sunday, August 02, 2015 10​:19 PM
To​: OtherRecipients of perl Ticket #125669​:
Cc​: perl5-porters@​perl.org
Subject​: [perl #125669] op/pack.t failures with PPC long double (double
double) builds

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?

Yes.

Along the same lines, when you build an unthreaded perl, do you get the
same failures?

Yes.

Cheers,
Rob

@p5pRT
Copy link
Author

p5pRT commented Aug 6, 2015

From @jhi

Looking 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<)) {
  "For list ($max_neg, -1, 0, 1, $max_pos) (total -1) packed with $x unpack '%65$x' gave 0, expected 36893488147419103231"
}

So it's the checksum computing pack/unpack, and for 65 bits. (The previous checksum sizes, including 63 and 64, have passed the tests.)

@p5pRT
Copy link
Author

p5pRT commented Aug 6, 2015

From @jhi

My suspicion hovers somewhere around line 1760 in pp_pack.c, this spot​:

...
  if (checksum) {
  if (strchr("fFdD", TYPE_NO_MODIFIERS(datumtype)) ||
  (checksum > bits_in_uv &&
  strchr("cCsSiIlLnNUWvVqQjJ", TYPE_NO_MODIFIERS(datumtype))) ) {
  NV trouble, anv;

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))'
3.68934881474190705e+19

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?

@p5pRT
Copy link
Author

p5pRT commented Aug 6, 2015

From [Unknown Contact. See original ticket]

My suspicion hovers somewhere around line 1760 in pp_pack.c, this spot​:

...
  if (checksum) {
  if (strchr("fFdD", TYPE_NO_MODIFIERS(datumtype)) ||
  (checksum > bits_in_uv &&
  strchr("cCsSiIlLnNUWvVqQjJ", TYPE_NO_MODIFIERS(datumtype))) ) {
  NV trouble, anv;

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))'
3.68934881474190705e+19

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?

@p5pRT
Copy link
Author

p5pRT commented Aug 6, 2015

From @sisyphus

-----Original Message-----
From​: Jarkko Hietaniemi via RT
Sent​: Thursday, August 06, 2015 12​:42 PM
To​: OtherRecipients of perl Ticket #125669​:
Cc​: perl5-porters@​perl.org
Subject​: [perl #125669] op/pack.t failures with PPC long double (double
double) builds

My suspicion hovers somewhere around line 1760 in pp_pack.c, this spot​:

...
if (checksum) {
if (strchr("fFdD", TYPE_NO_MODIFIERS(datumtype)) ||
(checksum > bits_in_uv &&
strchr("cCsSiIlLnNUWvVqQjJ",
TYPE_NO_MODIFIERS(datumtype))) ) {
NV trouble, anv;

Sisyphys, could you add

PerlIO_printf(Perl_debug_log, "cdouble = %"NVgf"\n", cdouble);

and the same for anv, whenever they have been updated?

I might not get to that until the weekend.

In Perl terms, e.g.

./perl -wle 'print unpack("%65s",pack("s*",-32768,-1,0,1,32764))'
3.68934881474190705e+19

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))'
36893488147419070464

The 3...e+19 is the 36893488147419103231 the pack.t was expecting, but got
zero.

I note that the value I got is much closer to the value you got than to the
expected "36893488147419103231".

The test 694 failure diagnostics tell me​:
# Failed test 694 - at op/pack.t line 628
# For list (-32768, -1, 0, 1, 32767) (total -1) packed with s unpack '%65s'
gave 0, expected 36893488147419103231
# numbers test for s>

How do I then replicate that failure once perl has been installed ?
I've tried​:

$ perl -wle 'print unpack("%65s",pack("s*",-32768, -1, 0, 1, 32767))'
36893488147419070464

$ perl -wle 'print unpack("%65s",pack("s>",-32768, -1, 0, 1, 32767))'
36893488147419070464

$ perl -wle 'print unpack("%65s",pack("s",-32768, -1, 0, 1, 32767))'
36893488147419070464

Just what is the actual command that's returning the '0' ?
Is one really expected to derive the answer to that question by digging
through pack.t ?

Of especial suspicion is the modf call in there. Does that work right for
these double-doubles?

Perl_modf() seems ok to me.
The attached Perl_modf.pl (which actually calls Perl_modf) outputs​:

1.41421356237309504880168872421
1
0.4142135623730950488016887242097

4.123105625617660549821409855974
4
0.1231056256176605498214098559741

Cheers,
Rob

@p5pRT
Copy link
Author

p5pRT commented Aug 6, 2015

From @sisyphus

Perl_modf.pl

@p5pRT
Copy link
Author

p5pRT commented Aug 6, 2015

From @jhi

The 3...e+19 is the 36893488147419103231 the pack.t was expecting, but
got zero.

Umm. Now this is strange (on a x86_64 system)​:

perl -wle 'print unpack("%65s*",pack("s*",-32768,-1,0,1,32767))'
0

(as opposed to '%64'​: perl -wle 'print unpack("%64s*",pack("s*",-32768,-1,0,1,32767))'
18446744073709551615)

So it seems the 0 is the really expected value with %65? If this is the case, then pack.t
is wrong in $calc_sum computation (line 600 and around). Maybe the line 711 is wrong in double-double?

711 if ($calc_sum == $calc_sum - 1 && $calc_sum == $max_p1) {

Sisyphus, could you debug some in pack.t?

@p5pRT
Copy link
Author

p5pRT commented Aug 6, 2015

From [Unknown Contact. See original ticket]

The 3...e+19 is the 36893488147419103231 the pack.t was expecting, but
got zero.

Umm. Now this is strange (on a x86_64 system)​:

perl -wle 'print unpack("%65s*",pack("s*",-32768,-1,0,1,32767))'
0

(as opposed to '%64'​: perl -wle 'print unpack("%64s*",pack("s*",-32768,-1,0,1,32767))'
18446744073709551615)

So it seems the 0 is the really expected value with %65? If this is the case, then pack.t
is wrong in $calc_sum computation (line 600 and around). Maybe the line 711 is wrong in double-double?

711 if ($calc_sum == $calc_sum - 1 && $calc_sum == $max_p1) {

Sisyphus, could you debug some in pack.t?

@p5pRT
Copy link
Author

p5pRT commented Aug 9, 2015

From @sisyphus

-----Original Message-----
From​: Jarkko Hietaniemi via RT
Sent​: Friday, August 07, 2015 9​:02 AM
To​: OtherRecipients of perl Ticket #125669​:
Cc​: perl5-porters@​perl.org
Subject​: [perl #125669] op/pack.t failures with PPC long double (double
double) builds

711 if ($calc_sum == $calc_sum - 1 && $calc_sum == $max_p1) {

Sisyphus, could you debug some in pack.t?

Yes, I think that line (line 611) makes assumptions that just don't hold for
doubledoubles.

I've verified that the condition ($calc_sum == $calc_sum - 1 && $calc_sum ==
$max_p1) is not met at any stage up to (and including) test 13348.
Test 13348 is the last test that fails - and I haven't checked beyond that
point to see if the condition is ever met, but I doubt that is as $max_p1 is
36893488147419103232 ( == 2 **65).
I've yet to calculate the lowest positive value for $calc_sum such that
$calc_sum == $calc_sum - 1. (Might be trivial - though I wouldn't be
surprised if the range of such values is *not* continuous.)

Judging by the associated comments, the underlying assumption of line 611
seems to be that 'long double' precision will be no greater than 64 bits.

I notice that the failing tests have invariably given 0, with an expected
value of 36893488147419103231 (which is $calc_sum).

But when I run the actual failing test, I invariably get a value that is
neither 0 nor 36893488147419103231.
For example, the test 13348 diagnostics tell me that the test failed
because​:

# For list (-2147483648, -1, 0, 1, 2147483647) (total -1) packed with j<
unpack '%65j<' gave 0, expected 36893488147419103231

But when I run that code from the command line I get something entirely
different​:
$ ../perl -I../lib -wle 'print
unpack("%65j<",pack("j<",-2147483648,-1,0,1,2147483647));'
36893488145271619584

65-bit binary representation of 36893488145271619584 is​:
11111111111111111111111111111111110000000000000000000000000000000

(That's 34 set bits followed by 31 unset bits.)
It's the same for *all* of the failing tests.

Now .... it's very easy to get all of the pack.t tests to pass.
We just just alter that condition at line 611 of pack.t to​:

(($calc_sum == $calc_sum - 1 && $calc_sum == $max_p1) || $calc_sum >= 2 **
64)

I think that is (maybe) a fair enough rewrite of the condition - assuming
that​:
a) the precision of the checksum can be no greater than UV precision;
b) the precision of the UV can be no greater than 64 bits.

But, although pack.t then passes all tests, determining that the previously
failing tests are now returning 0 as expected, perl is, in fact, returning a
different value (namely, 36893488145271619584).

I deduce 2 things from this​:
1) pack.t isn't deriving the "gave" value directly from the command it wants
to check;
2) there's a bug in in the perl source (probably the same sort of assumption
that line 611 made) that's allowing the return of 65-bit values.

I'll post this off as is - though I hope to make further progress today.
Any comments/corrections/pointers thus far appreciated.

Questions​:
Is my assumption that the checksum must fit into a UV correct ?
If so, why isn't "%x" a fatal error for x greater than UV-precision ?

Cheers,
Rob

@p5pRT
Copy link
Author

p5pRT commented Aug 9, 2015

From @sisyphus

-----Original Message-----
From​: Jarkko Hietaniemi via RT
Sent​: Thursday, August 06, 2015 12​:42 PM
To​: OtherRecipients of perl Ticket #125669​:
Cc​: perl5-porters@​perl.org
Subject​: [perl #125669] op/pack.t failures with PPC long double (double
double) builds

My suspicion hovers somewhere around line 1760 in pp_pack.c, this spot​:

...
if (checksum) {
if (strchr("fFdD", TYPE_NO_MODIFIERS(datumtype)) ||
(checksum > bits_in_uv &&
strchr("cCsSiIlLnNUWvVqQjJ",
TYPE_NO_MODIFIERS(datumtype))) ) {
NV trouble, anv;

As suggested, I replaced that section with​:

if (checksum) {
  if (strchr("fFdD", TYPE_NO_MODIFIERS(datumtype)) ||
  (checksum > bits_in_uv &&
  strchr("cCsSiIlLnNUWvVqQjJ", TYPE_NO_MODIFIERS(datumtype))) ) {
  NV trouble, anv;

  anv = (NV) (1 << (checksum & 15));
  PerlIO_printf(Perl_debug_log, "A anv = %"NVgf"\n", anv);
  while (checksum >= 16) {
  checksum -= 16;
  anv *= 65536.0;
  PerlIO_printf(Perl_debug_log, "B anv = %"NVgf"\n", anv);
  }
  while (cdouble < 0.0) {
  PerlIO_printf(Perl_debug_log, "A cdouble = %"NVgf"\n",
cdouble);
  cdouble += anv;
  PerlIO_printf(Perl_debug_log, "B cdouble = %"NVgf"\n",
cdouble);
  }
  cdouble = Perl_modf(cdouble / anv, &trouble) * anv;
  PerlIO_printf(Perl_debug_log, "C cdouble = %"NVgf"\n", cdouble);
  sv = newSVnv(cdouble);
  }

I then get​:

$ ./perl -I../lib -wle 'print
unpack("%65j<",pack("j<",-2147483648,-1,0,1,2147483647));'
A anv = 2
B anv = 131072
B anv = 8.58993e+09
B anv = 5.6295e+14
B anv = 3.68935e+19
A cdouble = -2.14748e+09
B cdouble = 3.68935e+19
C cdouble = 3.68935e+19
36893488145271619584

By my calculations, the final value for anv (3.68935e+19) is, in fact,
36893488147419103232 ( == 2 **65).
Apparently, the initial value of cdouble (-2.14748e+09) is, in fact,
2147483648 ( == 2 ** 31) since
36893488147419103232 - 2147483648 gives the final output value of
36893488145271619584 (which is also the final value of cdouble output
above).

Cheers,
Rob

@p5pRT
Copy link
Author

p5pRT commented Aug 11, 2015

From @sisyphus

-----Original Message-----
From​: sisyphus1@​optusnet.com.au
Sent​: Sunday, August 09, 2015 3​:28 PM

I notice that the failing tests have invariably given 0, with an expected
value of 36893488147419103231 (which is $calc_sum).

But when I run the actual failing test, I invariably get a value that is
neither 0 nor 36893488147419103231.
For example, the test 13348 diagnostics tell me that the test failed
because​:

# For list (-2147483648, -1, 0, 1, 2147483647) (total -1) packed with j<
unpack '%65j<' gave 0, expected 36893488147419103231

But when I run that code from the command line I get something entirely
different​:
$ ../perl -I../lib -wle 'print
unpack("%65j<",pack("j<",-2147483648,-1,0,1,2147483647));'
36893488145271619584

Ooops ... the actual "gave" value is determined a few lines higher up in
pack.t​:

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
unpack("%65j<*",pack("j<*",-2147483648,-1,0,1,2147483647));'
0

Which does, in fact, agree with the "gave" value !!
(It's a pity the diagnostic didn't mention the "*" ... but it's also a pity
that I wasn't paying better attention.)

We just need to alter the pack.t code so that the calculation of the
"expected" value is 0.

I was thinking again of my earlier suggestion of changing​:

if ($calc_sum == $calc_sum - 1 && $calc_sum == $max_p1) {
to​:
if (($calc_sum == $calc_sum - 1 && $calc_sum == $max_p1) || $calc_sum >= 2
** 64) {
or, to limit this to doubledouble builds​:
if($calc_sum == $calc_sum - 1 && $calc_sum == $max_p1) || ($calc_sum >= 2 **
64 && $Config{longdblkind} == 6)) {

BUT ... one problem with that solution is that, if pack.t were ever to test
the output of
unpack("%65j<",pack("j<",-2147483648,-1,0,1,2147483647))
then that test would fail - because pack.t would see a "gave" value of
36893488145271619584, but would have calculated the "expected" value to be
0.

I'll see if I can come up with a patch to pack.t such that the tests would
pass for both
unpack("%65j<",pack("j<",-2147483648,-1,0,1,2147483647))
and
unpack("%65j<*",pack("j<*",-2147483648,-1,0,1,2147483647))

Incidentally, the lowest value I've found for $x such that $x == $x-1 is
(2**107) - ((2**53)+1).
But there are numerous values greater than that for which the condition does
*not* hold.
I don't think the condition holds for any values lower than that. (Not yet
proven to my satisfaction.)

Cheers,
Rob

@p5pRT
Copy link
Author

p5pRT commented Aug 11, 2015

From @sisyphus

-----Original Message-----
From​: sisyphus1@​optusnet.com.au
Sent​: Tuesday, August 11, 2015 2​:22 PM
To​: perlbug-comment@​perl.org
Cc​: perl5-porters@​perl.org
Subject​: Re​: [perl #125669] op/pack.t failures with PPC long double (double
double) builds

I'll see if I can come up with a patch to pack.t such that the tests would
pass for both
unpack("%65j<",pack("j<",-2147483648,-1,0,1,2147483647))
and
unpack("%65j<*",pack("j<*",-2147483648,-1,0,1,2147483647))

Disregard all that I've written previously.
It turns out that there's a bug in my gcc/libc modfl() implementation, and
that bug carries through to Perl_modf().

At values sufficiently close to 1 and -1 (both above and below), modfl() and
Perl_modf() do the wrong thing.
Instead of returning a fractional value and setting the pointed-to-value to
the integer part, they do the converse.
That is, they set the pointed-to-value to the actual (fractional) value of
the given argument, and return 0.
When this happens In the op/pack.t tests, 0 is returned and the fractional
value is slightly less than +1.

The attached patch to pp_pack.c works around the problem in so far as it
allows pack.t to pass without any need to amend that file.

First question is, do we bother about this ?
If so, then the next question is, do we use the patch as is ? or should the
inclusion of this additional code be dependent upon a preprocessor directive
?

With this patch in place, I still get (as before)​:

../perl -I../lib -le 'print
unpack("%65j<",pack("j<",-2147483648,-1,0,1,2147483647));'
36893488145271619584

But if I go so far as to change line 570 of pack.t from​:

my $sum = eval {unpack "%$_$format*", pack "$format*", @​_};
to​:
my $sum = eval {unpack "%$_$format", pack "$format", @​_};

I get lots of failures and a hang. (Does it make sense to run such tests ?)

Cheers,
Rob

@p5pRT
Copy link
Author

p5pRT commented Aug 11, 2015

From @sisyphus

pp_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 {

@p5pRT
Copy link
Author

p5pRT commented Aug 20, 2015

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.
Remarkably, you've made the same mistakes as I did when I first tried to
catch and fix the errors.

Firstly, Perl_modf() can get it wrong for values greater than 1. The problem
with Perl_modf() is not limited to values less than 1 - in fact the same
problem can also arise for values around -1 (both above and below), too.

I know of (essentially) only one problem value - namely plus or minus
36893488147419103231 / 36893488147419103232 and reciprocals.
I haven't gone looking to see whether any other values cause Perl_modf to
misbehave, but I assume that if there other such problem values then they
fit the same pattern of behaviour.

Case A
Perl_modf(36893488147419103231 / 36893488147419103232, &trouble);
This should return its argument and set trouble to 0, but it does the
opposite of returning 0 and setting trouble to its argument.

Case B
Perl_modf(36893488147419103232 / 36893488147419103231, &trouble);
This should return its argument minus 1, and set trouble to 1. But it
returns 0 and sets trouble to its argument.

Case C (-ve version of Case A)
Perl_modf(- 36893488147419103231 / 36893488147419103232, &trouble);
This should return its argument and set trouble to 0, but it does the
opposite of returning 0 and setting trouble to its argument.

Case D (-ve version of Case B)
Perl_modf(-36893488147419103232 / 36893488147419103231, &trouble);
This should return its argument plus 1, and set trouble to -1. But it
returns 0 and sets trouble to its argument.

Now, as regards, op/pack.t, the only case we actually hit is that of Case A.
I suspect that it might be possible to compose a checksum example that hits
Case B - and if that's the case then I think the patch should allow for it.

Is it possible to strike Case C or Case D in the calculation of checksums ?
(ie can the checksum ever be negative ?)

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
Perl_modf() * anv. (If cdouble does need to be fixed, we need to fix it
*before* it gets multiplied by anv.)

Also, you're seeking to detect cdouble != Perl_ceil(cdouble), but that just
tells us that cdouble is non-integer.
We need to detect the case where cdouble is integer, but ought to be
non-integer - ie cdouble == Perl_ceil(cdouble) && trouble !=
Perl_ceil(trouble).
In fact, I think we don't even have to look at cdouble - it should be
sufficient to detect that trouble is non-integer (and that's what the latest
version of my patch now does).

I like the inclusion of the LONGDOUBLE_DOUBLEDOUBLE condition, so I've added
this to my patch.
(You might want to add some more elaborate explanation of what's happening
to the comments contained therein.)

The attached pp_pack.c_patch is working fine for me - though there are
currently no tests in the perl test suite that determine whether either of
these 2 lines are doing anything useful​:

+ if(cdouble > 1.0L) cdouble -= 1.0L;
+ 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
(resp) are ever encountered. (Cases A and C don't need either of those
conditions.)
Feel free to remove those 2 lines if you want - they're absence/presence has
no bearing on any current tests, AFAIK.

In my patch, it occurs to me that​:

  cdouble *= anv;
  sv = newSVnv(cdouble);

might better be replaced with​:

  sv = newSVnv(cdouble * anv);

but that would leave cdouble hanging with a different value. (I don't think
it would matter, but I decided to shy away from doing that, just in case.)

I re-ran 'make test' in its entirety after building with the attached patch,
and there were no new failures.

@p5pRT
Copy link
Author

p5pRT commented Aug 21, 2015

From @jhi

Fix now in as http​://perl5.git.perl.org/perl.git/commit/09b94b1f0efd8c107548a6fefcd471e9b06c2cdf

@p5pRT p5pRT closed this as completed Aug 21, 2015
@p5pRT
Copy link
Author

p5pRT commented Aug 21, 2015

@jhi - Status changed from 'open' to 'resolved'

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

No branches or pull requests

1 participant