On Sat, Jul 18, 2015 at 09:51:30PM -0700, Eliot Miranda wrote:
On Sat, Jul 18, 2015 at 8:10 PM, David T. Lewis lewis@mail.msen.com wrote:
On Sat, Jul 18, 2015 at 06:09:22PM -0700, Eliot Miranda wrote:
Hi David,
On Sat, Jul 18, 2015 at 3:49 PM, David T. Lewis lewis@mail.msen.com
wrote:
On Sat, Jul 18, 2015 at 08:55:27PM +0000, commits@source.squeak.org
wrote:
Eliot Miranda uploaded a new version of VMMaker to project VM Maker: http://source.squeak.org/VMMaker/VMMaker.oscog-eem.1426.mcz
==================== Summary ====================
Name: VMMaker.oscog-eem.1426 Author: eem Time: 18 July 2015, 1:54:29.051 pm UUID: 94ab92ba-c5c4-4953-8566-a4cd9c38df1f Ancestors: VMMaker.oscog-eem.1425
Fix at least one set of 64-bit issues caused by Slang. In particular
the integerObjectOf: code resulted in (objectMemory integerObjectOf: MillisecondClockMask) evaluating to the -1 object, instead of the 16r1FFFFFFF object, which was the cause of the initially nresponsive
64-bit
image on the real VM (no problem in the simulator).
I can't test now to verify, but I wonder if this change is fixing the right problem.
I'm pretty sure it is. It only bites in a 64-bit implementation with > 31-bit integers. In the "standard" 64-bit image SmallIntegers are still only 31-bits so the issue never occurs. The issue is that the default
type
of an integer constant in C is int. So if one has to shift any constant such that a non-zero bit will occupy bit 31 (0 relative), it must be cast to a long type to avoid sign extension.
I don't see it. The size of SmallInteger is enforced by the image, not by the object memory. There is nothing in the "standard" 64-bit image format that requires small integers to be restricted to the same limits imposed by a 32-bit object memory.
The #integerObjectOf: method answers a sqInt, which may be either 32 or 64 bits. This should just work, and I don't see a need for conditional logic other than what might be needed to define the size of sqInt in the first place.
Now of course I could generate all Integer constants with the L or UL suffix, e.g.
#define MillisecondClockMask 0x1FFFFFFFL
instead of
#define MillisecondClockMask 0x1FFFFFFF
but that's a much more pervasive change than only generating the cast in integerObjectOf when on 64-bits. So I'm happy with the change that I've made. Experience can of course prove me wrong...
I don't see the problem. Interpreter class>>initialize does this: MillisecondClockMask := 16r1FFFFFFF.
This works for all combinations or 32 and 64 bit image and host platform.
There should be no need to generate all of the integer constants with L or UL suffix. It is perfectly acceptable to assign 16r1FFFFFFF to a 64 bit signed or unsigned variable, and no special handling should be required.
I may be missing something, but I do not see anything about the Spur 64-bit image format that should require special handling for this.
Yes, I think you're missing something. let me take you through it. We're talking about evaluating, in a 64-bit C, sizeof(long) == 8, sizeof(int) == 4, the following two variants:
#define MillisecondClockMask 0x1FFFFFFF
(sqInt)((MillisecondClockMask << 3) + 1L)
(sqInt)(((sqInt)MillisecondClockMask << 3) + 1L)
So let's take the first. The type of MillisecondClockMask is int. The type of MillisecondClockMask << 3 is int. It's bit-pattern is 0xFFFFFFF8, and hence its value is, incorrectly, -8. So it evaluates to the SmallInteger -1, not 16r1FFFFFFF as desired.
In the second, the type of MillisecondClockMask is int. The type of (sqInt) MillisecondClockMask is long. The type of ((sqInt)MillisecondClockMask << 3). It's bit pattern is also 0xFFFFFFF8 but its value is 0xFFFFFFF8, /not/ -8, and so it evaluates to the SMallInteger 16r1FFFFFFF as desired.
Does that make sense?
Yes, the shift left 3 (rather that 1 in the old object format) is what I was missing.
I suspect that a cast to usqInt would accomplish the same thing without the ifTrue: test. Sorry I can't try it right now but it might be worth a look.
Dave