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

[David T. Lewis]

Eliot,

Thanks for this explanation. I had not spotted the discrepancy in the
sqComplexEvent struct. Yikes! no way that could work.

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
>