<div dir="ltr">Hi David,<div><br></div><div> 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.</div><div><br></div></div><div class="gmail_extra"><br><div class="gmail_quote">On Sat, Sep 6, 2014 at 8:45 PM, <span dir="ltr"><<a href="mailto:commits@source.squeak.org" target="_blank">commits@source.squeak.org</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><br>
David T. Lewis uploaded a new version of VMMaker to project VM Maker:<br>
<a href="http://source.squeak.org/VMMaker/VMMaker-dtl.350.mcz" target="_blank">http://source.squeak.org/VMMaker/VMMaker-dtl.350.mcz</a><br>
<br>
==================== Summary ====================<br>
<br>
Name: VMMaker-dtl.350<br>
Author: dtl<br>
Time: 6 September 2014, 11:44:28.389 pm<br>
UUID: 2ad132b0-5fb3-4580-b5a8-29af12c3cb81<br>
Ancestors: VMMaker-dtl.349<br>
<br>
VMMaker 4.13.6<br>
<br>
Fix primitiveBitShift (primitive 17) for sqInt declared 64 bits, as in a VM for 64-bit image format 68002.<br>
<br>
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.<br>
<br>
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).<br>
<br>
With this update, all KernelTests-Numbers tests pass for the 64-bit image format 68002.<br>
<br>
=============== Diff against VMMaker-dtl.349 ===============<br>
<br>
Item was changed:<br>
----- Method: InterpreterPrimitives>>primitiveBitShift (in category 'arithmetic integer primitives') -----<br>
primitiveBitShift<br>
| integerReceiver integerArgument shifted |<br>
+ <var: #shifted type: 'unsigned'><br>
+<br>
integerArgument := self popInteger.<br>
integerReceiver := self popPos32BitInteger.<br>
self successful ifTrue: [<br>
integerArgument >= 0 ifTrue: [<br>
"Left shift -- must fail if we lose bits beyond 32"<br>
self success: integerArgument <= 31.<br>
shifted := integerReceiver << integerArgument.<br>
self success: (shifted >> integerArgument) = integerReceiver.<br>
] ifFalse: [<br>
"Right shift -- OK to lose bits"<br>
self success: integerArgument >= -31.<br>
shifted := integerReceiver >> (0 - integerArgument).<br>
].<br>
].<br>
self successful<br>
ifTrue: [self push: (self positive32BitIntegerFor: shifted)]<br>
ifFalse: [self unPop: 2]!<br>
<br>
Item was changed:<br>
----- Method: VMMaker class>>versionString (in category 'version testing') -----<br>
versionString<br>
<br>
"VMMaker versionString"<br>
<br>
+ ^'4.13.6'!<br>
- ^'4.13.7'!<br>
<br>
</blockquote></div><br><br clear="all"><div><br></div>-- <br>best,<div>Eliot</div>
</div>