<div dir="ltr">Hi David,<div class="gmail_extra"><br><div class="gmail_quote">On Sun, Jul 19, 2015 at 8:52 PM, David T. Lewis <span dir="ltr"><<a href="mailto:lewis@mail.msen.com" target="_blank">lewis@mail.msen.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin-top:0px;margin-right:0px;margin-bottom:0px;margin-left:0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex"><span class=""><br>
On Sun, Jul 19, 2015 at 11:35:46AM -0400, David T. Lewis wrote:<br>
><br>
> On Sat, Jul 18, 2015 at 09:51:30PM -0700, Eliot Miranda wrote:<br>
> ><br>
> > On Sat, Jul 18, 2015 at 8:10 PM, David T. Lewis <<a href="mailto:lewis@mail.msen.com">lewis@mail.msen.com</a>> wrote:<br>
> > ><br>
</span><span class="">> > > I may be missing something, but I do not see anything about the Spur<br>
> > > 64-bit image format that should require special handling for this.<br>
> > ><br>
> ><br>
> > Yes, I think you're missing something. let me take you through it. We're<br>
> > talking about evaluating, in a 64-bit C, sizeof(long) == 8, sizeof(int) ==<br>
> > 4, the following two variants:<br>
> ><br>
> > #define MillisecondClockMask 0x1FFFFFFF<br>
> ><br>
> > (sqInt)((MillisecondClockMask << 3) + 1L)<br>
> ><br>
> > (sqInt)(((sqInt)MillisecondClockMask << 3) + 1L)<br>
> ><br>
> ><br>
> > So let's take the first. The type of MillisecondClockMask is int. The<br>
> > type of MillisecondClockMask << 3 is int. It's bit-pattern is 0xFFFFFFF8,<br>
> > and hence its value is, incorrectly, -8. So it evaluates to the<br>
> > SmallInteger -1, not 16r1FFFFFFF as desired.<br>
> ><br>
> > In the second, the type of MillisecondClockMask is int. The type of<br>
> > (sqInt) MillisecondClockMask is long. The type<br>
> > of ((sqInt)MillisecondClockMask << 3). It's bit pattern is also 0xFFFFFFF8<br>
> > but its value is 0xFFFFFFF8, /not/ -8, and so it evaluates to the<br>
> > SMallInteger 16r1FFFFFFF as desired.<br>
> ><br>
> > Does that make sense?<br>
><br>
> Yes, the shift left 3 (rather that 1 in the old object format) is what I<br>
> was missing.<br>
><br>
> I suspect that a cast to usqInt would accomplish the same thing without<br>
> the ifTrue: test. Sorry I can't try it right now but it might be worth a<br>
> look.<br>
><br>
<br>
</span>At the risk of further annoyance, here is one more question/observation:<br>
<br>
In VMMaker.oscog-eem.1417 (and sq.h in platforms sources), we change the<br>
type of the event buffer from int[8] to long[8], which corrects a failure<br>
in starting in the 64-bit Spur image. I don't know exactly what was failing,<br>
but I am suspecting perhaps the time stamp field in the event struct was<br>
being incorrectly converted from its 32 bit int value to an integer object<br>
in primitiveGetNextEvent, and that using long rather than int fields<br>
prevented this from happening.<br>
<br>
Later, in VMMaker.oscog-eem.1426 we have the fix for conversion of integer<br>
value to integer object, which particularly affected conversion of<br>
millisecond clock values. So I wonder if the fix in VMMaker.oscog-eem.1426<br>
might make the changes in VMMaker.oscog-eem.1417 unnecessary? And if so,<br>
can the fields of the sqInputEvent struct be changed back from long to<br>
int (possibly with some performance advantage)?<br></blockquote><div><br></div><div>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:</div><div><br></div><div>The generic input event conforms to int evtBuf[8]:</div><div><br></div><div><div>/* generic input event */</div><div>typedef struct sqInputEvent</div><div>{</div><div> int type; /* type of event; either one of EventTypeXXX */</div><div> unsigned int timeStamp; /* time stamp */</div><div> /* the interpretation of the following fields depend on the type of the event */</div><div> int unused1;</div><div> int unused2;</div><div> int unused3;</div><div> int unused4;</div><div> int unused5;</div><div> int windowIndex; /* SmallInteger used in image to identify a host window structure */</div><div>} sqInputEvent;</div></div><div><br></div><div>But the complex event *does not*:</div><div><br></div><div><div>typedef struct sqComplexEvent</div><div> {</div><div> int type; /* type of event; EventTypeComplex */</div><div> unsigned int timeStamp; /* time stamp */</div><div> /* the interpretation of the following fields depend on the type of the event */</div><div> int action; /* one of ComplexEventXXX (see below) */</div><div> usqInt objectPointer; /* used to point to object */</div><div> int unused1; /* */</div><div> int unused2; /* */</div><div> int unused3; /* */</div><div> int windowIndex; /* host window structure */</div><div> } sqComplexEvent;</div></div><div><br></div><div>In 64-bits usqInt objectPointer occupies 64-bits, not 32, and so a sqComplexEvent does not conform to int evtBuf[8].</div><div><br></div><div><br></div><div>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.</div><div><br></div><div>HTH</div><div><br></div></div><div class="gmail_signature"><div dir="ltr"><div><span style="font-size:small;border-collapse:separate"><div>_,,,^..^,,,_<br></div><div>best, Eliot</div></span></div></div></div>
</div></div>