primitiveHighBit interesting bug

Previous Topic Next Topic
 
classic Classic list List threaded Threaded
5 messages Options
Reply | Threaded
Open this post in threaded view
|

primitiveHighBit interesting bug

Nicolas Cellier
 
Hi all,
I got this on uptodate Mingw Win 64 bits Squeak.cog.spur VM:

(1to: 8) collect: [:i|-2942842961920 highBitOfMagnitude]
-> #(6 6 42 42 42 42 42 42)

That is correct once jitted, but wrong first two times...
I just can't understand when/how it fails by reading the code...
Any clue?

Nicolas
Reply | Threaded
Open this post in threaded view
|

Re: primitiveHighBit interesting bug

Nicolas Cellier
 
Ah! Got it!
The generated code is incorrect!
Source:

            leadingZeroCount = 0
                ifTrue:
                    ["highBit is not defined for negative Integer"
                    self primitiveFail]
                ifFalse:
                    ["Nice bit trick: 1-based high-bit is (32 - clz) -
1 to account for tag bit.
                    This is like two-complement - clz - 1 on 5 bits,
or in other words a bit-invert operation clz ^16r1F"
                    self pop: 1 thenPushInteger: (leadingZeroCount
bitXor: (BytesPerWord * 8 - 1))].
            ^self].

Generated:

        if (leadingZeroCount == 0) {

                /* highBit is not defined for negative Integer */
                /* begin primitiveFail */
                if (!GIV(primFailCode)) {
                        GIV(primFailCode) = 1;
                }
        }
        /* begin pop:thenPushInteger: */
        longAtput((sp = GIV(stackPointer) + ((0) * BytesPerWord)),
(((usqInt)(leadingZeroCount ^ ((BytesPerWord * 8) - 1)) << 3) | 1));
        GIV(stackPointer) = sp;

The ifFalse: branch has been gobbled...
Hence with always push (0 bitXor: 16r3F) as small integer in place of
the receiver, the fail the primitive...
The fallback code proceeds with 16r3F and answers 6... Correct.

WE MUST FIX THE GENERATOR ASAP !

Le ven. 15 janv. 2021 à 03:15, Nicolas Cellier
<[hidden email]> a écrit :

>
> Hi all,
> I got this on uptodate Mingw Win 64 bits Squeak.cog.spur VM:
>
> (1to: 8) collect: [:i|-2942842961920 highBitOfMagnitude]
> -> #(6 6 42 42 42 42 42 42)
>
> That is correct once jitted, but wrong first two times...
> I just can't understand when/how it fails by reading the code...
> Any clue?
>
> Nicolas
Reply | Threaded
Open this post in threaded view
|

Re: primitiveHighBit interesting bug

Eliot Miranda-2
 
I’ll fix it tomorrow morning.

Thanks!!

_,,,^..^,,,_ (phone)

> On Jan 14, 2021, at 6:36 PM, Nicolas Cellier <[hidden email]> wrote:
>
> 
> Ah! Got it!
> The generated code is incorrect!
> Source:
>
>            leadingZeroCount = 0
>                ifTrue:
>                    ["highBit is not defined for negative Integer"
>                    self primitiveFail]
>                ifFalse:
>                    ["Nice bit trick: 1-based high-bit is (32 - clz) -
> 1 to account for tag bit.
>                    This is like two-complement - clz - 1 on 5 bits,
> or in other words a bit-invert operation clz ^16r1F"
>                    self pop: 1 thenPushInteger: (leadingZeroCount
> bitXor: (BytesPerWord * 8 - 1))].
>            ^self].
>
> Generated:
>
>        if (leadingZeroCount == 0) {
>
>                /* highBit is not defined for negative Integer */
>                /* begin primitiveFail */
>                if (!GIV(primFailCode)) {
>                        GIV(primFailCode) = 1;
>                }
>        }
>        /* begin pop:thenPushInteger: */
>        longAtput((sp = GIV(stackPointer) + ((0) * BytesPerWord)),
> (((usqInt)(leadingZeroCount ^ ((BytesPerWord * 8) - 1)) << 3) | 1));
>        GIV(stackPointer) = sp;
>
> The ifFalse: branch has been gobbled...
> Hence with always push (0 bitXor: 16r3F) as small integer in place of
> the receiver, the fail the primitive...
> The fallback code proceeds with 16r3F and answers 6... Correct.
>
> WE MUST FIX THE GENERATOR ASAP !
>
>> Le ven. 15 janv. 2021 à 03:15, Nicolas Cellier
>> <[hidden email]> a écrit :
>>
>> Hi all,
>> I got this on uptodate Mingw Win 64 bits Squeak.cog.spur VM:
>>
>> (1to: 8) collect: [:i|-2942842961920 highBitOfMagnitude]
>> -> #(6 6 42 42 42 42 42 42)
>>
>> That is correct once jitted, but wrong first two times...
>> I just can't understand when/how it fails by reading the code...
>> Any clue?
>>
>> Nicolas
Reply | Threaded
Open this post in threaded view
|

Re: primitiveHighBit interesting bug

Eliot Miranda-2
In reply to this post by Nicolas Cellier
 
Hi Nicolas,

On Thu, Jan 14, 2021 at 6:36 PM Nicolas Cellier <[hidden email]> wrote:
 
Ah! Got it!
The generated code is incorrect!
Source:

            leadingZeroCount = 0
                ifTrue:
                    ["highBit is not defined for negative Integer"
                    self primitiveFail]
                ifFalse:
                    ["Nice bit trick: 1-based high-bit is (32 - clz) -
1 to account for tag bit.
                    This is like two-complement - clz - 1 on 5 bits,
or in other words a bit-invert operation clz ^16r1F"
                    self pop: 1 thenPushInteger: (leadingZeroCount
bitXor: (BytesPerWord * 8 - 1))].
            ^self].


The source I have looks like:

                      "Note: in gcc, result is undefined if input is zero (for compatibility with BSR fallback when no CLZ instruction available).
but input is never zero because we pass the oop with tag bits set, so we are safe"
objectMemory wordSize = 4
ifTrue: [leadingZeroCount := self __builtin_clz: integerReceiverOop]
ifFalse: [leadingZeroCount := self __builtin_clzll: integerReceiverOop].
leadingZeroCount = 0 ifTrue: "highBit is not defined for negative Integer"
[self primitiveFail].
"Nice bit trick: 1-based high-bit is (32 - clz) - 1 to account for tag bit.
This is like two-complement - clz - 1 on 5 bits, or in other words a bit-invert operation clz ^16r1F"
self pop: 1 thenPushInteger: (leadingZeroCount bitXor: (BytesPerWord * 8 - 1))
so the problem is not in the generator.  Did you fix this and not publish?

Generated:

        if (leadingZeroCount == 0) {

                /* highBit is not defined for negative Integer */
                /* begin primitiveFail */
                if (!GIV(primFailCode)) {
                        GIV(primFailCode) = 1;
                }
        }
        /* begin pop:thenPushInteger: */
        longAtput((sp = GIV(stackPointer) + ((0) * BytesPerWord)),
(((usqInt)(leadingZeroCount ^ ((BytesPerWord * 8) - 1)) << 3) | 1));
        GIV(stackPointer) = sp;

The ifFalse: branch has been gobbled...
Hence with always push (0 bitXor: 16r3F) as small integer in place of
the receiver, the fail the primitive...
The fallback code proceeds with 16r3F and answers 6... Correct.

WE MUST FIX THE GENERATOR ASAP !

Le ven. 15 janv. 2021 à 03:15, Nicolas Cellier
<[hidden email]> a écrit :
>
> Hi all,
> I got this on uptodate Mingw Win 64 bits Squeak.cog.spur VM:
>
> (1to: 8) collect: [:i|-2942842961920 highBitOfMagnitude]
> -> #(6 6 42 42 42 42 42 42)
>
> That is correct once jitted, but wrong first two times...
> I just can't understand when/how it fails by reading the code...
> Any clue?
>
> Nicolas


--
_,,,^..^,,,_
best, Eliot
Reply | Threaded
Open this post in threaded view
|

Re: primitiveHighBit interesting bug

Nicolas Cellier
 
Hi Eliot,

Le ven. 15 janv. 2021 à 06:44, Eliot Miranda <[hidden email]> a écrit :

>
>
> Hi Nicolas,
>
> On Thu, Jan 14, 2021 at 6:36 PM Nicolas Cellier <[hidden email]> wrote:
>>
>>
>> Ah! Got it!
>> The generated code is incorrect!
>> Source:
>>
>>             leadingZeroCount = 0
>>                 ifTrue:
>>                     ["highBit is not defined for negative Integer"
>>                     self primitiveFail]
>>                 ifFalse:
>>                     ["Nice bit trick: 1-based high-bit is (32 - clz) -
>> 1 to account for tag bit.
>>                     This is like two-complement - clz - 1 on 5 bits,
>> or in other words a bit-invert operation clz ^16r1F"
>>                     self pop: 1 thenPushInteger: (leadingZeroCount
>> bitXor: (BytesPerWord * 8 - 1))].
>>             ^self].
>>
>
> The source I have looks like:
>
>                       "Note: in gcc, result is undefined if input is zero (for compatibility with BSR fallback when no CLZ instruction available).
> but input is never zero because we pass the oop with tag bits set, so we are safe"
> objectMemory wordSize = 4
> ifTrue: [leadingZeroCount := self __builtin_clz: integerReceiverOop]
> ifFalse: [leadingZeroCount := self __builtin_clzll: integerReceiverOop].
> leadingZeroCount = 0 ifTrue: "highBit is not defined for negative Integer"
> [self primitiveFail].
> "Nice bit trick: 1-based high-bit is (32 - clz) - 1 to account for tag bit.
> This is like two-complement - clz - 1 on 5 bits, or in other words a bit-invert operation clz ^16r1F"
> self pop: 1 thenPushInteger: (leadingZeroCount bitXor: (BytesPerWord * 8 - 1))
> so the problem is not in the generator.  Did you fix this and not publish?
>

Oups, too many VMMaker images,
I picked the source from the wrong one,
so this is not a generator bug, just a refactoring slip indeed.
Thanks for the quick fix.
Note that the bug might have altered the printString of some negative Float...
It might be the regression that you were tracking by the end of last year...

cheers

>> Generated:
>>
>>         if (leadingZeroCount == 0) {
>>
>>                 /* highBit is not defined for negative Integer */
>>                 /* begin primitiveFail */
>>                 if (!GIV(primFailCode)) {
>>                         GIV(primFailCode) = 1;
>>                 }
>>         }
>>         /* begin pop:thenPushInteger: */
>>         longAtput((sp = GIV(stackPointer) + ((0) * BytesPerWord)),
>> (((usqInt)(leadingZeroCount ^ ((BytesPerWord * 8) - 1)) << 3) | 1));
>>         GIV(stackPointer) = sp;
>>
>> The ifFalse: branch has been gobbled...
>> Hence with always push (0 bitXor: 16r3F) as small integer in place of
>> the receiver, the fail the primitive...
>> The fallback code proceeds with 16r3F and answers 6... Correct.
>>
>> WE MUST FIX THE GENERATOR ASAP !
>>
>> Le ven. 15 janv. 2021 à 03:15, Nicolas Cellier
>> <[hidden email]> a écrit :
>> >
>> > Hi all,
>> > I got this on uptodate Mingw Win 64 bits Squeak.cog.spur VM:
>> >
>> > (1to: 8) collect: [:i|-2942842961920 highBitOfMagnitude]
>> > -> #(6 6 42 42 42 42 42 42)
>> >
>> > That is correct once jitted, but wrong first two times...
>> > I just can't understand when/how it fails by reading the code...
>> > Any clue?
>> >
>> > Nicolas
>
>
>
> --
> _,,,^..^,,,_
> best, Eliot