Switch to new BigInt implementation #997
Conversation
|
Well, this is a messy one... we had already axed BigFloat, BigDecimal, use math and qjscalc. In addition some of our changes conflict so I'm pretty much surgically applying each hunk by hand. Opening as draft to signal it's WIP. |
|
Only major thing left is Math.sumPrecise. I hope to crack that one tonight... |
|
Ah, you might want to leave that one alone for now (and keep libbf a little longer) because you can't get the necessary precision with plain doubles (or even I did some work on it a while ago (basically a bespoke floating point adder with ample precision) but I haven't had time to finish it. |
|
The internal functions seem to suffice (at a cursory look, very la the last night though!) except they cannot represent NaN and Infinity so some checks would be necessary and maybe to keep state as the spec describes. I'll give it a shot and see if it can work 😅 |
|
@saghul: |
|
Ah FML, I see what you guys meant now. The new internal functions only work for ints, not floats, so the addition doesn't work. I'll look into keeping libbf around a bit longer then. |
Not sure what you mean.
This function can be implemented with regular IEEE doubles, but the algorithm is subtle. Keeping libbf for this sole purpose seems overkill. |
|
Alright, I think I got it all tidy up. This was very painful to do. In order to implement Let's see what the CI has to say. @bnoordhuis can you give it a read-through when you get a chance? You can likely skip the large chunks of contiguous code that are directly applied from bellard/ but I had to make some tweaks here and there. |
saghul
force-pushed
the
new-bigint
branch
2 times, most recently
from
fb929f3 to
10dc80f
Compare
Looks like we need to link with clang RT, at a cursory search... something relates to int128 operations. |
Ah I think we can fix it by using the same trick as we did in libbf and use 32 bits for the limbs on Windows. |
Manually port the following commits from bellard/quickjs bellard/quickjs@61e8b94 bellard/quickjs@ee4cd4d bellard/quickjs@6de8885 bellard/quickjs@5a16c0c bellard/quickjs@6d6893b bellard/quickjs@131408f Co-authored-by: Fabrice Bellard <[email protected]> Since libbf is gone, incorporate xsum as the backing for the Math.sumPrecise implementation. Changes made to xsum: - Removed printf debug statements since many contained type errors - Removed unneeded debug functions - Added incldue guards
There was a problem hiding this comment.
LGTM, good job. A couple of observations:
-
such a missed opportunity to name it JS_TAG_LITTLE_BIG_INT
-
for another PR: maybe remove support for 32 bits limbs (or vice versa, 64 bits limbs); simplifies the code a great deal in some places. Likely doesn't affect real world performance much because hardly anyone uses real big BigInts
-
fwiw, firefox/spidermonkey uses xsum_small_accumulator in Math.sumPrecise. xsum_large_accumulator takes up over 8k stack space, which is maybe a little much.
| ret = false; | ||
| for(i = p->len - 1; i >= 0; i--) { | ||
| if (p->tab[i] != 0) { | ||
| ret = true; | ||
| break; | ||
| } | ||
| } | ||
| JS_FreeValue(ctx, val); | ||
| return ret; |
There was a problem hiding this comment.
Not wrong but can be simplified to:
| ret = false; | |
| for(i = p->len - 1; i >= 0; i--) { | |
| if (p->tab[i] != 0) { | |
| ret = true; | |
| break; | |
| } | |
| } | |
| JS_FreeValue(ctx, val); | |
| return ret; | |
| for(i = p->len - 1; i >= 0; i--) | |
| if (p->tab[i]) | |
| break; | |
| JS_FreeValue(ctx, val); | |
| return i >= 0; |
If it's verbatim from bellard then just keep it as-is.
|
Thanks for the insight Ben! I'll look into those in follow-up PRs since this one is already quite big 😅 |
No description provided.