This is a patch version of what I indicated in May 2013:
--- GMP.xs.old 2015-12-01 16:10:02.000000000 -0800
+++ GMP.xs 2015-12-01 19:04:39.000000000 -0800
@@ -169,7 +169,7 @@
/* using the IV directly is a bit faster */
if (SvUOK(x))
{
- mpz_init_set_si(*RETVAL, (UV)SvUV(x));
+ mpz_init_set_ui(*RETVAL, (UV)SvUV(x));
}
else
{
@@ -190,7 +190,7 @@
CODE:
mpz = malloc (sizeof(mpz_t));
if (SvUOK(x)) {
- mpz_init_set_si(*mpz, (UV)SvUV(x));
+ mpz_init_set_ui(*mpz, (UV)SvUV(x));
}
else {
mpz_init_set_str(*mpz, SvPV_nolen(x), 10);
Math::BigInt's new() strips the sign off the input, so anything coming to _new is unsigned. The defect is that _new is coercing the UV into a signed long.
Regarding performance, unfortunately since Math::BigInt removes the sign by using s/// that means any signed input comes in looking like a string so goes through the slower string path. Probably not a huge problem since most inputs are likely unsigned.
The following is a bit more efficient, as it results in many more integer inputs going through the faster path:
--- GMP.xs.old 2015-12-01 16:10:02.000000000 -0800
+++ GMP.xs 2015-12-01 19:11:28.000000000 -0800
@@ -167,9 +167,9 @@
CODE:
NEW_GMP_MPZ_T;
/* using the IV directly is a bit faster */
- if (SvUOK(x))
+ if (SvUOK(x) || SvIOK(x))
{
- mpz_init_set_si(*RETVAL, (UV)SvUV(x));
+ mpz_init_set_ui(*RETVAL, (UV)SvUV(x));
}
else
{
@@ -189,8 +189,8 @@
mpz_t *mpz;
CODE:
mpz = malloc (sizeof(mpz_t));
- if (SvUOK(x)) {
- mpz_init_set_si(*mpz, (UV)SvUV(x));
+ if (SvUOK(x) || SvIOK(x)) {
+ mpz_init_set_ui(*mpz, (UV)SvUV(x));
}
else {
mpz_init_set_str(*mpz, SvPV_nolen(x), 10);
All current tests pass with this patch. It is, of course, possible that the test suite isn't complete.
I'm also attaching a test file. When run with Calc or Pari back end, it passes all tests in both 32-bit and 64-bit Perl. With the current GMP back end, it fails 4 tests and hangs or dies on a fifth without the patch. This test exercises code I run every day, but have to work around by forcing all arguments to new() into strings, just in case the GMP back end is being used and the input falls into the bad range. With either patch above, this passes all tests just like the other back ends.
I don't think this test file is ready to get dropped into the test suite, but portions of it may be appropriate to add. I think the tests for:
-(~0 >> 1 + 1)
~0 >> 1
~0 >> 1 + 1
~0 - 1
~0
would be good to see.
#!/usr/bin/env perl
use strict;
use warnings;
use Test::More;
use Math::BigInt lib=>"GMP";
my $use64 = ~0 > 4294967295;
my $broken64 = (18446744073709550592 == ~0);
die "Your 64-bit system is broken. Upgrade from 5.6 for this test." if $broken64;
plan tests => 4*2 + 2*1 + 1 + $use64;
my $maxs = ~0 >> 1;
for my $n ($maxs - 2, $maxs - 1, $maxs, $maxs+1) {
is( Math::BigInt->new($n), $n, "new $n" );
is( Math::BigInt->new(-$n), -$n, "new -$n" );
}
for my $n (~0 - 1, ~0) {
is( Math::BigInt->new($n), $n, "new $n" );
}
# bacmp makes a new variable. This will test if it is screwing up the sign.
is( Math::BigInt->new(10)->bacmp(~0), -1, "10 should be less than maxint" );
if ($use64) {
diag "The following test may hang or cause an exception if incorrect";
is( Math::BigInt->new("14")->bmodpow(9506577562092332135, "29544731879021791655795710"), "19946192910281559497582964", "big modpow" );
}
