-
Notifications
You must be signed in to change notification settings - Fork 7.9k
Enable 64bit integer support on 32bit arch --enable-zend-int64
#19079
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
base: master
Are you sure you want to change the base?
Conversation
I think this should go through the RFC process. There have been suggestions to drop 32-bit support, so I'm sure at least some people would object to expanding support further. I could see this as an alternative to dropping 32-bit support iff we also dropped 4-byte zend_long support, as that would align 32 and 64-bit. |
void gmp_set_zlong(mpz_t z, zend_long zlong) { | ||
if (zlong <= GMP_SI_MAX && zlong >= GMP_SI_MIN) { | ||
mpz_set_si(z, zlong); | ||
} else if (zlong >= 0) { | ||
mpz_import(z, 1, 1, sizeof(zend_long), 0, 0, &zlong); | ||
} else { | ||
mpz_import(z, 1, 1, sizeof(zend_long), 0, 0, &zlong); | ||
mpz_neg(z, z); | ||
} | ||
} | ||
|
||
int gmp_fits_zlong_p(mpz_t z) { | ||
int result = 1; | ||
mpz_t z_min_max; | ||
zend_long min_max; | ||
|
||
if (mpz_cmp_si(z, GMP_SI_MAX) > 0) { | ||
min_max = ZEND_LONG_MAX; | ||
mpz_init(z_min_max); | ||
mpz_import(z_min_max, 1, 1, sizeof(zend_long), 0, 0, &min_max); | ||
result = mpz_cmp(z, z_min_max) <= 0; | ||
mpz_clear(z_min_max); | ||
} else if (mpz_cmp_si(z, GMP_SI_MIN) < 0) { | ||
min_max = ZEND_LONG_MIN; | ||
mpz_init(z_min_max); | ||
mpz_import(z_min_max, 1, 1, sizeof(zend_long), 0, 0, &min_max); | ||
mpz_neg(z_min_max, z_min_max); | ||
result = mpz_cmp(z, z_min_max) >= 0; | ||
mpz_clear(z_min_max); | ||
} | ||
|
||
return result; | ||
} | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can these functions be static?
I have fixed some cases which failed due to SIZEOF_SIZE_T != SIZEOF_ZEND_LONG, but there are probably more. Currently, the test state is as follows:
|
Yes, of course. At the current state I want to get to know the work needed to be done and do some benchmarks. For dropping 32-bit support entirely it seems to be to early at least for WebAssembly + there might be people hoping for supporting older (in some cases not that old) systems - so I don't thing it would go through currently. |
3c8a5ac
to
97bb3dd
Compare
This PR allows to use 64bit integers on 32bit platforms if compiled with
--enable-zend-int64
--enable-zend-int64
)PHP_SYS_SIZE = [4|8]
)Zend/tests/
andtests/
ext/
The main part is that
ZEND_ENABLE_ZVAL_LONG64 1
gets defined.