[Vm-dev] VM Maker: VMMaker.oscog-nice.2723.mcz
Nicolas Cellier
nicolas.cellier.aka.nice at gmail.com
Tue Mar 10 21:14:50 UTC 2020
Hi Levente,
for some reason, early VM writers were looking for a Logical Right Shift
(not propagating the sign bit).
So that's how bitShift: (then >>) were historically translated - no matter
how weird or contradictory to VM simulation it can be.
I guess that it is of greatest interest for generating BitBlt operations
(Simulation works ok because WordArray are like unsigned).
Eliot had to later introduce >>> for Arithmetic Right Shift and
signedBitShift: too, because we sometimes need those operations too.
That makes one more surprise because we now have the exact opposite of Java
semantics for >> and >>> !
We have another similar surprising behavior with translation of // and \\
which are translated into C operations equivalent to quo: and rem:
https://smallissimo.blogspot.com/2015/03/is-bitshift-equivalent-to-division-in.html?m=0
These hackish choices are unfortunate, but very difficult to change now
without hiccups...
Le mar. 10 mars 2020 à 21:55, Levente Uzonyi <leves at caesar.elte.hu> a
écrit :
>
> Hi Nicolas,
>
> Thanks for the fix. It works.
> I wonder why is it necessary to cast to usqInt. Is it just to ensure
> unsignedness?
>
>
> Levente
>
> On Mon, 9 Mar 2020, commits at source.squeak.org wrote:
>
> >
> > Nicolas Cellier uploaded a new version of VMMaker to project VM Maker:
> > http://source.squeak.org/VMMaker/VMMaker.oscog-nice.2723.mcz
> >
> > ==================== Summary ====================
> >
> > Name: VMMaker.oscog-nice.2723
> > Author: nice
> > Time: 10 March 2020, 12:26:31.04183 am
> > UUID: c1319382-406c-43a7-9f55-2b48c4007d80
> > Ancestors: VMMaker.oscog-eem.2722
> >
> > Fix the right shift: dont convert to usqInt a type longer than usqInt
> >
> > =============== Diff against VMMaker.oscog-eem.2722 ===============
> >
> > Item was changed:
> > ----- Method: CCodeGenerator>>generateBitShift:on:indent: (in category
> 'C translation') -----
> > generateBitShift: msgNode on: aStream indent: level
> > "Generate the C code for this message onto the given stream."
> >
> > + | arg shift rightShift |
> > + (self isConstantNode: (arg := msgNode args first) valueInto:
> [:shiftValue| shift := shiftValue])
> > - | arg rcvr shift |
> > - arg := msgNode args first.
> > - rcvr := msgNode receiver.
> > - (self isConstantNode: arg valueInto: [:shiftValue| shift :=
> shiftValue])
> > ifTrue: "bit shift amount is a constant"
> > + [aStream nextPut: $(.
> > - [aStream nextPutAll: '((usqInt) '.
> > - self emitCExpression: rcvr on: aStream.
> > shift < 0
> > + ifTrue:
> > + [rightShift := TSendNode new
> > + setSelector: #>>
> > + receiver: msgNode receiver
> > + arguments: {TConstantNode
> new setValue: shift negated}.
> > + self generateShiftRight:
> rightShift on: aStream indent: level]
> > + ifFalse: [self generateShiftLeft: msgNode
> on: aStream indent: level].
> > - ifTrue: [aStream nextPutAll: ' >> ';
> print: shift negated]
> > - ifFalse: [aStream nextPutAll: ' << ';
> print: shift].
> > aStream nextPut: $)]
> > ifFalse: "bit shift amount is an expression"
> > + [rightShift := TSendNode new
> > + setSelector: #>>
> > + receiver: msgNode receiver
> > + arguments: {TSendNode new
> > + setSelector:
> #negated
> > + receiver: arg
> > + arguments: #()}.
> > + aStream nextPutAll: '(('.
> > + self emitCExpression: arg on: aStream.
> > + aStream nextPutAll: ' < 0) ? ('.
> > + self generateShiftRight: rightShift on: aStream
> indent: level.
> > + aStream nextPutAll: ') : ('.
> > + self generateShiftLeft: msgNode on: aStream
> indent: level.
> > - [aStream nextPutAll: '(('.
> > - self emitCExpression: arg on: aStream indent:
> level.
> > - aStream nextPutAll: ' < 0) ? ((usqInt) '.
> > - self emitCExpression: rcvr on: aStream indent:
> level.
> > - aStream nextPutAll: ' >> -'.
> > - self emitCExpression: arg on: aStream indent:
> level.
> > - aStream nextPutAll: ') : ((usqInt) '.
> > - self emitCExpression: rcvr on: aStream indent:
> level.
> > - aStream nextPutAll: ' << '.
> > - self emitCExpression: arg on: aStream indent:
> level.
> > aStream nextPutAll: '))']!
> >
> > Item was changed:
> > ----- Method: CCodeGenerator>>generateShiftRight:on:indent: (in
> category 'C translation') -----
> > generateShiftRight: msgNode on: aStream indent: level
> > + "Generate the C code for this message onto the given stream.
> > + Note that this generates a Logical Shift (unsigned), not an
> Arithmetic Shift (signed)."
> > - "Generate the C code for this message onto the given stream."
> >
> > + | type typeIsUnsigned mustCastToUnsigned unsignedType |
> > + type := self typeFor: msgNode receiver in: currentMethod.
> > + typeIsUnsigned := type first = $u.
> > + mustCastToUnsigned := typeIsUnsigned not or:
> > + ["cast to usqInt if the int is shorter: we want to avoid
> UB related to a shift exceeeding bit width"
> > + (self sizeOfIntegralCType: type) < (self
> sizeOfIntegralCType: #usqInt)].
> > + "If not unsigned cast it to unsigned."
> > + mustCastToUnsigned
> > - | type |
> > - "If the variable is a 64-bit type then don't cast it to usqInt
> (typically a 32-bit type)"
> > - (self is64BitIntegralVariable: msgNode receiver typeInto: [:t|
> type := t])
> > ifTrue:
> > + ["If the variable is a 64-bit type then don't cast
> it to usqInt (typically a 32-bit type)"
> > + unsignedType := (self sizeOfIntegralCType: type) <
> (self sizeOfIntegralCType: #usqLong)
> > + ifTrue: [#usqInt]
> > + ifFalse: [self
> unsignedTypeForIntegralType: type].
> > + aStream nextPutAll: '(('; nextPutAll:
> unsignedType; nextPutAll: ')('.
> > - ["If not unsigned cast it to unsigned."
> > - type first ~= $u ifTrue:
> > - [aStream nextPutAll: '((unsigned ';
> nextPutAll: type; nextPut: $)].
> > self emitCExpression: msgNode receiver on:
> aStream indent: level.
> > + aStream nextPutAll: '))']
> > - type first ~= $u ifTrue:
> > - [aStream nextPut: $)]]
> > ifFalse:
> > + [aStream nextPutAll: '('.
> > - [aStream nextPutAll: '((usqInt) '.
> > self emitCExpression: msgNode receiver on:
> aStream indent: level.
> > aStream nextPut: $)].
> > aStream nextPutAll: ' >> '.
> > self emitCExpression: msgNode args first on: aStream indent: level!
> >
> > Item was changed:
> > ----- Method: CCodeGenerator>>generateSignedBitShift:on:indent: (in
> category 'C translation') -----
> > generateSignedBitShift: msgNode on: aStream indent: level
> > "Generate the C code for this message onto the given stream."
> >
> > + | arg shift rightShift |
> > - | cast type arg shift |
> > - "since ``signed'' is a synonym for ``signed int'' do not cast
> 64-bit values to signed if at all possible."
> > - cast := (self is64BitIntegralVariable: msgNode receiver typeInto:
> [:t| type := t])
> > - ifTrue: ['(', (type first = $u ifTrue:
> [type allButFirst: (type second = $n ifTrue: [2] ifFalse: [1])] ifFalse:
> [type]), ')']
> > - ifFalse: ['(signed)'].
> > (self isConstantNode: (arg := msgNode args first) valueInto:
> [:shiftValue| shift := shiftValue])
> > ifTrue: "bit shift amount is a constant"
> > + [aStream nextPut: $(.
> > - [aStream nextPut: $(; nextPutAll: cast.
> > - self emitCExpression: msgNode receiver on: aStream.
> > shift < 0
> > + ifTrue:
> > + [rightShift := TSendNode new
> > + setSelector: #>>
> > + receiver: msgNode receiver
> > + arguments: {TConstantNode
> new setValue: shift negated}.
> > + self generateSignedShiftRight:
> rightShift on: aStream indent: level]
> > + ifFalse: [self generateShiftLeft: msgNode
> on: aStream indent: level].
> > - ifTrue: [aStream nextPutAll: ' >> ';
> print: shift negated]
> > - ifFalse: [aStream nextPutAll: ' << ';
> print: shift].
> > aStream nextPut: $)]
> > ifFalse: "bit shift amount is an expression"
> > + [rightShift := TSendNode new
> > + setSelector: #>>
> > + receiver: msgNode receiver
> > + arguments: {TSendNode new
> > + setSelector:
> #negated
> > + receiver: arg
> > + arguments: #()}.
> > + aStream nextPutAll: '(('.
> > - [aStream nextPutAll: '(('.
> > self emitCExpression: arg on: aStream.
> > + aStream nextPutAll: ' < 0) ? ('.
> > + self generateSignedShiftRight: rightShift on:
> aStream indent: level.
> > + aStream nextPutAll: ') : ('.
> > + self generateShiftLeft: msgNode on: aStream
> indent: level.
> > - aStream nextPutAll: ' < 0) ? ('; nextPutAll: cast.
> > - self emitCExpression: msgNode receiver on: aStream.
> > - aStream nextPutAll: ' >> -'.
> > - self emitCExpression: arg on: aStream.
> > - aStream nextPutAll: ') : ('; nextPutAll: cast.
> > - self emitCExpression: msgNode receiver on: aStream.
> > - aStream nextPutAll: ' << '.
> > - self emitCExpression: arg on: aStream.
> > aStream nextPutAll: '))']!
> >
> > Item was changed:
> > ----- Method: CCodeGenerator>>generateSignedShiftRight:on:indent: (in
> category 'C translation') -----
> > generateSignedShiftRight: msgNode on: aStream indent: level
> > "Generate the C code for >>> onto the given stream."
> >
> > + | type typeIsUnsigned mustCastToSigned signedType |
> > + type := self typeFor: msgNode receiver in: currentMethod.
> > + typeIsUnsigned := type first = $u.
> > + mustCastToSigned := typeIsUnsigned or:
> > + ["cast to sqInt if the int is shorter: we want to avoid UB
> related to a shift exceeeding bit width"
> > + (self sizeOfIntegralCType: type) < (self
> sizeOfIntegralCType: #usqInt)].
> > + mustCastToSigned
> > - (self is64BitIntegralVariable: msgNode receiver typeInto: [:t|])
> > ifTrue:
> > + ["If the variable is a 64-bit type then don't cast
> it to usqInt (typically a 32-bit type)"
> > + signedType := (self sizeOfIntegralCType: type) <
> (self sizeOfIntegralCType: #usqLong)
> > + ifTrue: [#usqInt]
> > + ifFalse: [self signedTypeForIntegralType:
> type].
> > + aStream nextPutAll: '(('; nextPutAll: signedType;
> nextPutAll: ')('.
> > + self emitCExpression: msgNode receiver on:
> aStream indent: level.
> > + aStream nextPutAll: '))']
> > - [aStream nextPutAll: '((sqLong) ']
> > ifFalse:
> > + [aStream nextPutAll: '('.
> > + self emitCExpression: msgNode receiver on:
> aStream indent: level.
> > + aStream nextPut: $)].
> > + aStream nextPutAll: ' >> '.
> > - [aStream nextPutAll: '((sqInt) '].
> > - self emitCExpression: msgNode receiver on: aStream.
> > - aStream nextPutAll: ') >> '.
> > self emitCExpression: msgNode args first on: aStream!
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.squeakfoundation.org/pipermail/vm-dev/attachments/20200310/4b5fed2d/attachment-0001.html>
More information about the Vm-dev
mailing list