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

 

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, view it on GitHub, or mute the thread.

<script type="application/ld+json">[ { "@context": "http://schema.org", "@type": "EmailMessage", "potentialAction": { "@type": "ViewAction", "target": "https://github.com/OpenSmalltalk/opensmalltalk-vm/issues/415?email_source=notifications\u0026email_token=AIJPEW3JURXUBOPFWOJWRLDQFQO3RA5CNFSM4IN2ACMKYY3PNVWWK3TUL52HS4DFUVEXG43VMWVGG33NNVSW45C7NFSM4HGJTTMA", "url": "https://github.com/OpenSmalltalk/opensmalltalk-vm/issues/415?email_source=notifications\u0026email_token=AIJPEW3JURXUBOPFWOJWRLDQFQO3RA5CNFSM4IN2ACMKYY3PNVWWK3TUL52HS4DFUVEXG43VMWVGG33NNVSW45C7NFSM4HGJTTMA", "name": "View Issue" }, "description": "View this Issue on GitHub", "publisher": { "@type": "Organization", "name": "GitHub", "url": "https://github.com" } } ]</script>

 

The failures are unrelated, we can close

—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub, or mute the thread.

<script type="application/ld+json">[ { "@context": "http://schema.org", "@type": "EmailMessage", "potentialAction": { "@type": "ViewAction", "target": "https://github.com/OpenSmalltalk/opensmalltalk-vm/issues/415?email_source=notifications\u0026email_token=AIJPEW234IAXWTMKFP777CLQFXB5NA5CNFSM4IN2ACMKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOD43LYRQ#issuecomment-523680838", "url": "https://github.com/OpenSmalltalk/opensmalltalk-vm/issues/415?email_source=notifications\u0026email_token=AIJPEW234IAXWTMKFP777CLQFXB5NA5CNFSM4IN2ACMKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOD43LYRQ#issuecomment-523680838", "name": "View Issue" }, "description": "View this Issue on GitHub", "publisher": { "@type": "Organization", "name": "GitHub", "url": "https://github.com" } } ]</script>

 

Closed #415.

—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub, or mute the thread.

<script type="application/ld+json">[ { "@context": "http://schema.org", "@type": "EmailMessage", "potentialAction": { "@type": "ViewAction", "target": "https://github.com/OpenSmalltalk/opensmalltalk-vm/issues/415?email_source=notifications\u0026email_token=AIJPEW2RONDATTUD6UGI473QFXB5NA5CNFSM4IN2ACMKYY3PNVWWK3TUL52HS4DFWZEXG43VMVCXMZLOORHG65DJMZUWGYLUNFXW5KTDN5WW2ZLOORPWSZGOTF3UUQY#event-2574731843", "url": "https://github.com/OpenSmalltalk/opensmalltalk-vm/issues/415?email_source=notifications\u0026email_token=AIJPEW2RONDATTUD6UGI473QFXB5NA5CNFSM4IN2ACMKYY3PNVWWK3TUL52HS4DFWZEXG43VMVCXMZLOORHG65DJMZUWGYLUNFXW5KTDN5WW2ZLOORPWSZGOTF3UUQY#event-2574731843", "name": "View Issue" }, "description": "View this Issue on GitHub", "publisher": { "@type": "Organization", "name": "GitHub", "url": "https://github.com" } } ]</script>

 

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

—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub, or mute the thread.

<script type="application/ld+json">[ { "@context": "http://schema.org", "@type": "EmailMessage", "potentialAction": { "@type": "ViewAction", "target": "https://github.com/OpenSmalltalk/opensmalltalk-vm/issues/415?email_source=notifications\u0026email_token=AIJPEW5DCNYXLPZA3Z2VGDDQGGTSDA5CNFSM4IN2ACMKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOD5CIAIQ#issuecomment-524582946", "url": "https://github.com/OpenSmalltalk/opensmalltalk-vm/issues/415?email_source=notifications\u0026email_token=AIJPEW5DCNYXLPZA3Z2VGDDQGGTSDA5CNFSM4IN2ACMKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOD5CIAIQ#issuecomment-524582946", "name": "View Issue" }, "description": "View this Issue on GitHub", "publisher": { "@type": "Organization", "name": "GitHub", "url": "https://github.com" } } ]</script>

 
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