hpi-swa / RSqueak

A Squeak/Smalltalk VM written in RPython.
BSD 3-Clause "New" or "Revised" License
83 stars 15 forks source link

#bitInvert32 broke with fast-bytes #138

Closed krono closed 7 years ago

krono commented 7 years ago

Squeak 64bit:

((2 raisedTo: 30) -1 ) in: [:x| 'x ', x, ' x inv ', x bitInvert32]
 'x 1073741823 x inv 3221225472'

RSqueak 64bit:

((2 raisedTo: 30) -1 ) in: [:x| 'x ', x, ' x inv ', x bitInvert32]
'x 1073741823 x inv -0000000003221225474'

Note #bitXor: is this in Integer

bitInvert32
    "Answer the 32-bit complement of the receiver."

    ^ self bitXor: 16rFFFFFFFF

This is uses excessively in BitBlt(Simulation). viz. BitBltSimulation >> #copyLoop:

copyLoop
    "…"
            destWord := (destMask bitAnd: mergeWord) bitOr:
                            (destWord bitAnd: destMask bitInvert32).
    "…"

which is translated to

// …
            destWord = (destMask & mergeWord) | (destWord & ((unsigned int)~destMask));

While this looks unproblematic on first sight (b/c uint will be 64bit on 64bit anyway), I'm unsure for all the other shifts

krono commented 7 years ago

Buglet:

RSqueak 64bit:

((2 raisedTo: 30) -1 ) bitInvert32. "-3221225474 [SmallInteger]"
" inlined #bitInvert32 "
((2 raisedTo: 30) -1 )  bitXor: 16rFFFFFFFF."3221225472 [SmallInteger]"
krono commented 7 years ago

Non-Buglet: RSqueak 32bit:

((2 raisedTo: 30) -1 ) bitInvert32. "3221225472 [SmallInteger]"
" inlined #bitInvert32 "
((2 raisedTo: 30) -1 )  bitXor: 16rFFFFFFFF."3221225472 [SmallInteger]"
timfel commented 7 years ago

I noticed that in the previous impl, we ignored the sign in the bit operations and forcibly interpreted the arguments as uints, whereas now we keep the sign intact (and go to rbigint, if required). One thing to try is to revert to the old behavior, and not use rbigint at all, maybe the fallback code does something that rbigint doesn't...

krono commented 7 years ago

Fixed via 4941d8f6d