[Vm-dev] urgent info required on Slang's shift treatment...

[Eliot Miranda]

On Tue, Mar 3, 2009 at 11:58 AM, John M McIntosh <
johnmci at smalltalkconsulting.com> wrote:

>
> Er, well the shift left/right assembler operations are signed or unsigned,
> but TYPED languages usually figure out which one to use based on the type of
> the operators.


Forget about assembler.  In C "signed >> expr" is an arithmetic (signed)
shift; "unsigned >> expr" is a logical (unsigned) one.  In Smalltalk
"negative bitShift: negative" is an arithmetic shift, "non-negative
bitShift: negative" is a logical one.  These are perfectly compatible, and
both are useful.  In Smalltalk you can always write
       (foo bitAnd: (1 bitShift: preShiftFieldWidth) - 1) bitShift: expr
if you want to perform a logical shift.

But since Smalltalk is type-less,


No, that's not it.  In fact, Smalltalk is strongly typed.  Try bit shifting
true.  Quite possible in C; not possible in Smalltalk.  Smalltalk has
infinite precision so an unsigned value is simply a non-negative one.  The
reason, I think, that Slang casts to unsigned is a hack to do with the
default type of oops being sqInt (signed).  But this is a mistake.  Look at
the definition of primitiveBitShift and ask yourself why this even works?

primitiveBitShift
| integerReceiver integerArgument shifted |
integerArgument := self popInteger.
integerReceiver := self popPos32BitInteger.
self successful ifTrue: [
integerArgument >= 0 ifTrue: [
"Left shift -- must fail if we lose bits beyond 32"
self success: integerArgument <= 31.
shifted := integerReceiver << integerArgument.
self success: (shifted >> integerArgument) = integerReceiver.
] ifFalse: [
"Right shift -- OK to lose bits"
self success: integerArgument >= -31.
shifted := integerReceiver bitShift: integerArgument.
].
].
self successful
ifTrue: [self push: (self positive32BitIntegerFor: shifted)]
ifFalse: [self unPop: 2]

It took me a few minutes of staring at
shifted := integerReceiver bitShift: integerArgument.
(which can't possibly work for negative values if Slang, as it does, casts
to unsigned) before I realised that

integerReceiver := self popPos32BitInteger.

means the primitive simply fails for negative receivers (!!!!!!!!!!!!!).

I've noticed this before and I've forgotten it (I'm a bit like a goldfish in
that regard) and I find it no less shocking each time I trip over it.

What if you code up a unsigned shift operator so there is no question that
> you want to do unsigned shifting, which also makes the  SLANG coder just a
> bit more aware he has a choice to make, casting is ok, but people forget.
> Also I can do senders of, then later wonder why am I not doing that
> here/there?


Right.  I've recently added #asUnsignedInteger as a concise cast to
unsigned.


Having been bitten by signed shifting issues in the past, I can agree it's
> unpleasant.




So look, surely shifts should be signed by default and be unsigned if the
argument is unsigned, e.g./i.e.

!Integer methodsFor: '*VMMaker-interpreter simulator' stamp: 'eem 2/19/2009
18:33'!
asUnsignedInteger
self assert: self >= 0.
^self! !

and in CCodeGenerator>>initializeCTranslationDictionary:
    ...
#asUnsignedInteger #generateAsUnsignedInteger:on:indent:
    ...

and
!CCodeGenerator methodsFor: 'C translation' stamp: 'eem 2/15/2009 16:38'!
generateAsUnsignedInteger: msgNode on: aStream indent: level
"Generate the C code for this message onto the given stream."

aStream nextPutAll:'((usqInt)'.
self emitCExpression: msgNode receiver on: aStream.
aStream nextPut: $)! !

and I autochange all senders of >> and bitShift: throughout the plugins,
primitives and Interpreter to use asUnsignedInteger.  Do I dare?  Am I
insane?


>
>
> PS I note we have methods  to  do unsigned compare logic when dealing with
> oop address so that *we* know we are doing something special, since
> people just couldn't manage the casts correctly.
>
>
> On 3-Mar-09, at 11:50 AM, Eliot Miranda wrote:
>
>  Hi All,
>>
>>    I'm being bitten by Slang's treatment of bitShift: & >>.  In both cases
>> (generateBitShift:on:indent: & generateShiftRight:on:indent:) Slang
>> generates an unsigned shift by explicitly casting the shifted expression to
>> usqInt.  I can understand the benefit of having an unsigned shift.  But
>> there are times when one really needs a signed shift.  Further, the
>> Smalltalk versions of both bitShift: and >> are signed shifts.
>>
>> Dare I change e.g. generateShiftRight:on:indent: to leave the expression
>> alone and generate either a signed or an unsigned shift based on the
>> variable's declaration?  Or must I live with a maddening cCode: '(signed)'
>> inSmalltalk: [] carbuncle?
>>
>> E.
>>
>
> --
> ===========================================================================
> John M. McIntosh <johnmci at smalltalkconsulting.com>
> Corporate Smalltalk Consulting Ltd.  http://www.smalltalkconsulting.com
> ===========================================================================
>
>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://lists.squeakfoundation.org/pipermail/vm-dev/attachments/20090303/a6ca02bc/attachment-0001.htm