the goal is to test if an integer value can fit in a 61bits Small Integer or not. The way to do it, is as in `isIntegerValue:`
We check that most significant 4 bits are all 0 or all 1. This is achieved by shifting `>> 60`. we should then have `xxxx0000 `or `xxxx1111` then we add `1` we should have `xxxx0001 `or `xxxx0000` we thus test: `& 0b1111 <= 1 `=> SmallInteger `& 0b1111 > 1` => NotSmallInteger
Unfortunately, the JIT have it right once:
genJumpIsSmallIntegerValue: aRegister scratch: scratchReg "Generate a test for aRegister containing an integer value in the SmallInteger range, and a jump if so, answering the jump. c.f. Spur64BitMemoryManager>>isIntegerValue:" <returnTypeC: #'AbstractInstruction *'> ^cogit MoveR: aRegister R: scratchReg; ArithmeticShiftRightCq: 63 - objectMemory numTagBits R: scratchReg; AddCq: 1 R: scratchReg; AndCq: 1 << (objectMemory numTagBits + 1) - 1 R: scratchReg; "sign and top numTags bits must be the same" CmpCq: 1 R: scratchReg; JumpLessOrEqual: 0
but wrong once:
genJumpNotSmallIntegerValue: aRegister scratch: scratchReg "Generate a test for aRegister containing an integer value outside the SmallInteger range, and a jump if so, answering the jump. c.f. Spur64BitMemoryManager>>isIntegerValue:" <returnTypeC: #'AbstractInstruction *'> ^cogit MoveR: aRegister R: scratchReg; ArithmeticShiftRightCq: 64 - objectMemory numTagBits R: scratchReg; AddCq: 1 R: scratchReg; AndCq: 1 << (objectMemory numTagBits + 1) - 1 R: scratchReg; "sign and top numTags bits must be the same" CmpCq: 1 R: scratchReg; JumpGreater: 0
64 should be 63, or we only test highest 3 bits...
Fortunately, this is only used in genPrimitiveDivide... Unfortunately, this triggers the `(self deny: SmallInteger minVal / -1 = SmallInteger minVal)` bug...
That's not the first time that this bug happens, I already reported it in the past, but it seems that we did not capitalize this test case (or it is not jitted?). If you accept a short method in Object:
testDiv | si | si := -1152921504606846976. ^si / -1
then force the jitter with a bench:
[self assert: 0 testDiv isLarge] bench
then you'll trigger it...
The failures are unrelated, we can close
Closed #415.
For the sake of archeology, I detected similar bug in 2012 for legacy interpreter See thread **[Vm-dev] 3 Bugs in LargeInteger primitives** http://lists.squeakfoundation.org/pipermail/vm-dev/2012-August/011161.html
On Sat, Aug 24, 2019 at 02:44:01PM -0700, Nicolas Cellier wrote:
For the sake of archeology, I detected similar bug in 2012 for legacy interpreter See thread **[Vm-dev] 3 Bugs in LargeInteger primitives** http://lists.squeakfoundation.org/pipermail/vm-dev/2012-August/011161.html
Thanks for these archeology links, it is very helpful.
Sorry I don't recall the history of this issue, but the bugs that you identified in 2012 for legacy interpreter do not seem to be present in the interpreter VM today. I am assuming this is because you fixed those issues in VMMaker afterwards, is that right?
Here is what I see on my system today (interpreter VM, Ubuntu Linux, gcc compiler):
((1<<63) negated quo: -1) hex. => '16r8000000000000000' ((1<<63) negated / -1) hex. => '16r8000000000000000' ((1<<63) negated * -1) hex. => '16r8000000000000000' ((1<<63) negated // -1) hex. => '16r8000000000000000'
Dave
vm-dev@lists.squeakfoundation.org