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

Implement all LargeInteger Primitives (20-39) #37

Open
codefrau opened this issue Jun 23, 2015 · 6 comments
Open

Implement all LargeInteger Primitives (20-39) #37

codefrau opened this issue Jun 23, 2015 · 6 comments

Comments

@codefrau
Copy link
Owner

codefrau commented Jun 23, 2015

... using 53 bit logic, as in e3627a0?
... and ideally, using JS BigInt numbers if we get outside the 53 bit range, instead of failing the prims.

@codefrau codefrau changed the title Implement LargeInteger Primitives (20-39) Implement all LargeInteger Primitives (20-39) Jun 23, 2015
@ErikOnBike
Copy link
Contributor

I needed integers a little bigger than default (in a 32 bit image). Therefore added a number of the LargeInteger primitives using the technique mentioned with 53-bit JS logic/support. If interested, you can find it here: ErikOnBike@8df8f91

@codefrau
Copy link
Owner Author

codefrau commented Jun 1, 2024

This breaks LargeInteger semantics. After cherry-picking ErikOnBike@8df8f91 Squeak fails to start up. I'm not entirely sure which of the large int ops actually leads to failure, but even just the first one is wrong:

[] in Delay class>>startTimerEventLoop
Delay class>>runTimerEventLoop
Delay class>>handleTimerEvent
Delay class>>scheduleDelay:from:
LargePositiveInteger>>+

ctx[5]=rcvr: 16r0DD63AD203E07F (3894722817155199L)
ctx[6]=tmp0/arg0: 500000

This is performing 3894722817155199 + 500000 = 3894723654997199 which indeed is in the Int53 range. However, your code does not actually convert the result to a LargeInteger and instead answers the result as a BoxedFloat64. But in Squeak, Integers are guaranteed to be accurate and must never automatically flow over into inaccurate Floats.

Instead of makeStObj() you might use pos53BitIntFor() which in the case of addition could do the Right Thing, if you properly guard against negative results. There probably should be a similar one that handles negative numbers too, I just have not had the need for that.

The other operations do not look quite correct either. E.g. division needs to fail if the result is not an integer, so that the image fallback code can construct a Fraction. And the various Rem/Mod/Quo operations have subtly different semantics for signed numbers - that's why even the SmallInteger code uses this.vm.quickDivide() , this.vm.mod(), this.vm.div() etc.

So while this may work for your specific purposes, my goal for SqueakJS is to provide a VM with perfectly accurate semantics, even if it comes with speed penalties.

@codefrau
Copy link
Owner Author

codefrau commented Jun 1, 2024

... that being said, implementing these primitives is still a Good Idea. For best speed I would even try to use JS BigInt numbers if we get outside the 53 bit range, instead of failing the prims in that case.

The reason I haven't done that yet is because the 53 bit stuff is only used for clock operations so far which are all positive. The most used LargeIntegers besides for clock purposes is full 32 bit operations, which is implemented by the various "pos32Bit" functions.

All other LargeInts are handled by the image fallback code which uses the LargeInteger plugin transpiled from Slang.

@ErikOnBike
Copy link
Contributor

Hmmm. Hadn't tried it on regular images. I'm using it with my tiny 32-bit image. The 53-bit support is already very welcome for things like a clock or some browser identifiers. I use a custom makeStObject so I might have different results. Your findings makes me think I should perform some additional tests though ;-). Thx

@ErikOnBike
Copy link
Contributor

I don't have Fractions in my image, so Float works for me. But I understand and agree SqueakJS in general should be fully compatible.

Small thing: I think the optimised "obj == (obj|0)" in makeStObject does not (or did not seem to) work in all cases. I replaced it with "Number.isInteger(obj)" which works for me. YMMV

@codefrau
Copy link
Owner Author

codefrau commented Jun 2, 2024

The line in makeStObject() is

if (obj === (obj|0)) return this.makeLargeIfNeeded(obj);
else return this.makeFloat(obj);

Since my makeLargeIfNeeded() only handles SmallIntegers and positive 32 bit LargeInts (it throws otherwise), the check needs to match that. And you're right, there are some negative SmallInts giving a false positive with my check, and some large positive ints giving a false negative.

If you were to use Number.isInteger() then you would still need to add upper and lower bounds checks to match makeLargeIfNeeded(), otherwise it will throw. So it would need to check for ≥ Squeak.MinSmallInt = -0x40000000 and ≤ 0xFFFFFFFF (max positive 32 bit LargeInt).

In practice, I've never observed makeLargeInt() throw, but you're right, that would be more accurate. And makeStObject() isn't used in performance-critical code so we may as well do that.

We could also make makeLargeInt() work for all JS integers (all Number.isInteger() == true). That could be fun. The reason I didn't was that it's only used for file and memory sizes, which are positive and 32 bits on the 32 bit system we're pretending to be.

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

No branches or pull requests

2 participants