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

[Eliot Miranda]

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
-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://lists.squeakfoundation.org/pipermail/vm-dev/attachments/20150720/e46bedc5/attachment.htm