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

David T. Lewis lewis at mail.msen.com
Tue Jul 21 02:04:47 UTC 2015


On Mon, Jul 20, 2015 at 04:48:35PM -0400, David T. Lewis wrote:
> Eliot,
> 
> Thanks for this explanation. I had not spotted the discrepancy in the
> sqComplexEvent struct. Yikes! no way that could work.

... except that sqComplexEvent does not appear to be used anywhere except
the iOS code base, either in oscog or trunk platform sources. The declaration
of sqComplexEvent was definitely not right, since it would work only for
the case of sizeof(sqInt) == 4. But I don't see how that would impact a 
VM that does not make any reference to sqComplexEvent.

I tried applying the change to make event fields long rather than int,
checking for the effect on a format 68002 image (64 bit, 64-bit interpreter
VM). Everything still works (as expected), but the annoying problem I have
been seeing (need to wiggle the mouse to keep background unit tests active,
hence my hunch that the issue is related to event structures) is not affected.

The original declaration of sqComplexEvent was wrong. The approach of
changing the fields from int to long would be a reasonable fix for that
problem. But I thing it's a red herring, and I still suspect that the
64-bit Spur image issue addressed in VMMaker.oscog-eem.1417 was actually
caused by time stamp conversion, and fixed in your later VMMaker.oscog-eem.1426
update.

I'm sorry I can't test to confirm, but I'll bet that if you revert the
sqInputEvent fields to be int (original declaration) rather than long,
the 64-bit Spur VM will still work just fine.

Dave


> 
> Thanks,
> Dave
> 
> >  Hi David,
> >
> > On Sun, Jul 19, 2015 at 8:52 PM, David T. Lewis <lewis at mail.msen.com>
> > wrote:
> >
> >>
> >> On Sun, Jul 19, 2015 at 11:35:46AM -0400, David T. Lewis wrote:
> >> >
> >> > 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:
> >> > > >
> >> > > > 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.
> >> >
> >>
> >> At the risk of further annoyance, here is one more question/observation:
> >>
> >> In VMMaker.oscog-eem.1417 (and sq.h in platforms sources), we change the
> >> type of the event buffer from int[8] to long[8], which corrects a
> >> failure
> >> in starting in the 64-bit Spur image. I don't know exactly what was
> >> failing,
> >> but I am suspecting perhaps the time stamp field in the event struct was
> >> being incorrectly converted from its 32 bit int value to an integer
> >> object
> >> in primitiveGetNextEvent, and that using long rather than int fields
> >> prevented this from happening.
> >>
> >> Later, in VMMaker.oscog-eem.1426 we have the fix for conversion of
> >> integer
> >> value to integer object, which particularly affected conversion of
> >> millisecond clock values. So I wonder if the fix in
> >> VMMaker.oscog-eem.1426
> >> might make the changes in VMMaker.oscog-eem.1417 unnecessary? And if so,
> >> can the fields of the sqInputEvent struct be changed back from long to
> >> int (possibly with some performance advantage)?
> >>
> >
> > Well, indeed the commit comment on  VMMaker.oscog-eem.1417 is wrong, in
> > that the size of the evtBuf wasn't the cause of the bug.  But there /is/ a
> > bug here with 64-bits.  Look at platforms/Cross/vm/sq.h:
> >
> > The generic input event conforms to int evtBuf[8]:
> >
> > /* generic input event */
> > typedef struct sqInputEvent
> > {
> >   int type;         /* type of event; either one of EventTypeXXX */
> >   unsigned int timeStamp;   /* time stamp */
> >   /* the interpretation of the following fields depend on the type of the
> > event */
> >   int unused1;
> >   int unused2;
> >   int unused3;
> >   int unused4;
> >   int unused5;
> >   int windowIndex;      /* SmallInteger used in image to identify a host
> > window structure */
> > } sqInputEvent;
> >
> > But the complex event *does not*:
> >
> > typedef struct sqComplexEvent
> >     {
> >         int type;           /* type of event;  EventTypeComplex */
> >         unsigned int timeStamp; /* time stamp */
> >         /* the interpretation of the following fields depend on the type
> >  of the event */
> >         int action;             /* one of ComplexEventXXX (see below) */
> >         usqInt objectPointer;   /* used to point to object */
> >         int unused1;            /*  */
> >         int unused2;            /*  */
> >         int unused3;            /*  */
> >         int windowIndex;    /* host window structure */
> >     } sqComplexEvent;
> >
> > In 64-bits usqInt objectPointer occupies 64-bits, not 32, and so
> > a sqComplexEvent does not conform to int evtBuf[8].
> >
> >
> > Hence my solution is to change evtBuf to long evtBuf[8], and change all
> > the
> > fields in the events from (unsigned) int to (unsigned) long.  There is no
> > performance issue with 32-bits; sizeof(long) == sizeof(int), and in 64
> > bits
> > it won't make any difference either; events are simply not that frequent
> > for this to be an issue.
> >
> > HTH
> >
> > _,,,^..^,,,_
> > best, Eliot
> >
> 


More information about the Vm-dev mailing list