[Vm-dev] VM Maker: VMMaker-dtl.350.mcz

[Eliot Miranda]

Hi David,

    thanks for this fix.  But IMNRHO you're trying to make a silk purse out
of a sow's ear.  The method is broken in not handling negative integers.
 IMO it should handle negative receivers, and handle negative shifts
properly.  Further, this "popInteger... unPop" idiom is ugly and
inefficient.  The one pop once the result is computed works better,
updating the stackPointer global only once with a write whose result is not
needed, instead of the multiple pops which depend on prior modifications of
stackPointer.  I'm in the middle of something right now so can't provide a
fix this instant.  But I'll try and rewrite it asap.


On Sat, Sep 6, 2014 at 8:45 PM, <commits at source.squeak.org> wrote:

>
> David T. Lewis uploaded a new version of VMMaker to project VM Maker:
> http://source.squeak.org/VMMaker/VMMaker-dtl.350.mcz
>
> ==================== Summary ====================
>
> Name: VMMaker-dtl.350
> Author: dtl
> Time: 6 September 2014, 11:44:28.389 pm
> UUID: 2ad132b0-5fb3-4580-b5a8-29af12c3cb81
> Ancestors: VMMaker-dtl.349
>
> VMMaker 4.13.6
>
> Fix primitiveBitShift (primitive 17) for sqInt declared 64 bits, as in a
> VM for 64-bit image format 68002.
>
> In image format 68002, bit shift left failed because of return type
> limited to a 32-bit large integer, but internally the primitive
> successfully shifted left into a variable declared as a 64 bit sqInt.. The
> simple fix (implemented here) is to declare the variable as 32 bit unsigned
> to agree with the 32-bit logic of the existing primitive.
>
> Note that permitting a left shift into a 64 bit variable makes sense
> generally, and the primitive could be recoded to accomodate this for shift
> left, with the primitive answering positive64BitIntegerFor: rather than
> positive32BitIntegerFor: in the case of a shift left to allow for the
> greater range (not implemented in this update).
>
> With this update, all KernelTests-Numbers tests pass for the 64-bit image
> format 68002.
>
> =============== Diff against VMMaker-dtl.349 ===============
>
> Item was changed:
>   ----- Method: InterpreterPrimitives>>primitiveBitShift (in category
> 'arithmetic integer primitives') -----
>   primitiveBitShift
>         | integerReceiver integerArgument shifted |
> +       <var: #shifted type: 'unsigned'>
> +
>         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 >> (0 -
> integerArgument).
>                 ].
>         ].
>         self successful
>                 ifTrue: [self push: (self positive32BitIntegerFor:
> shifted)]
>                 ifFalse: [self unPop: 2]!
>
> Item was changed:
>   ----- Method: VMMaker class>>versionString (in category 'version
> testing') -----
>   versionString
>
>         "VMMaker versionString"
>
> +       ^'4.13.6'!
> -       ^'4.13.7'!
>
>


-- 
best,
Eliot
-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://lists.squeakfoundation.org/pipermail/vm-dev/attachments/20140908/6b7cb16a/attachment.htm