codefrau / SqueakJS

A Squeak Smalltalk VM in Javascript
https://squeak.js.org
MIT License
371 stars 76 forks source link

Implement all LargeInteger Primitives (20-39) #37

Open codefrau opened 9 years ago

codefrau commented 9 years ago

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

ErikOnBike commented 4 months ago

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: https://github.com/ErikOnBike/SqueakJS/commit/8df8f91d6967dca113299d758911fd932d12340d

codefrau commented 4 months ago

This breaks LargeInteger semantics. After cherry-picking https://github.com/ErikOnBike/SqueakJS/commit/8df8f91d6967dca113299d758911fd932d12340d 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 commented 4 months ago

... 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 commented 4 months ago

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 commented 4 months ago

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 commented 4 months ago

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.