primitiveHighBit interesting bug

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

> 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

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