[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