[Vm-dev] [OpenSmalltalk/opensmalltalk-vm] incorrect Spur64 genJumpNotSmallIntegerValue:scratch: bugs SmallInteger minVal / -1 (#415)

Nicolas Cellier notifications at github.com
Tue Aug 20 16:56:24 UTC 2019


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...

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/OpenSmalltalk/opensmalltalk-vm/issues/415
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.squeakfoundation.org/pipermail/vm-dev/attachments/20190820/312e27b7/attachment.html>


More information about the Vm-dev mailing list