[Vm-dev] VM Maker: VMMaker.oscog-eem.1426.mcz

David T. Lewis lewis at mail.msen.com
Sun Jul 19 15:35:46 UTC 2015


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 at 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 at mail.msen.com>
> > wrote:
> > >
> > > >
> > > > On Sat, Jul 18, 2015 at 08:55:27PM +0000, commits at 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



More information about the Vm-dev mailing list