In message 266c38f11b5e7ad322b984b76df5bed4@inria.fr Ian Piumarta ian.piumarta@inria.fr wrote:

float *argPtr; ... /* Check if any of the argument's values is zero */

argPtr = ((float *) (interpreterProxy->firstIndexableField(arg))); for (i = 0; i <= (length - 1); i += 1) { if ((intAtPointer(argPtr + i)) == 0) { return interpreterProxy->primitiveFail(); }

Looks very plausible, I'll give it a try.

but that begs the question: why are we testing for integer 0 in an array of floats? Even if 0.0 happens to have all 0 bits, it would seem better to implement floatAtPointer() in sqMemoryAccess.h and compare against 0.0 (assuming the compiler and runtime agree on what constitutes floating 0 -- floating equality is a dodgy concept wherever it occurs).

My thoughts exactly but then I didn't write the code originally so have no idea what reasoning lead to it. According to a quick google, zero has e of 0, m of 0 and s-bit of 0 or 1, so simply testing for (int)0 is only catching half the possible corect values. So I suppose we really ought to be using something like intAtPointer(blah) & 0x7FFFFFFF == 0 ?

Who's the numerics afficionado now that Dave N Smith is no longer with us?

tim -- Tim Rowledge, tim@sumeru.stanford.edu, http://sumeru.stanford.edu/tim Oxymorons: Government organization

Hi Dave,

On Apr 29, 2005, at 09:08, Dave Mason wrote:

I assume that sqInt is defined as equivalent in size to a pointer (since that is the point),

Almost. sqInt is defined to be the same width as an object pointer. Because of the 4-way compatibility (you can run 32- or 64-bit images on 32- or 64-bit hardware) the width of an oop is not the same as the width of a raw pointer in 2 of the 4 cases.

The tricky case is running a 32-bit image on 64-bit hardware, where the object memory is almost certainly mapped way above the range of 32-bit addresses. To deal with this, sqMemoryAccess.h has macros to deal with translating 32-bit oops to/from 64-bit addresses, based on the address at which the object memory has been allocated by the OS. For 64-bit cleanliness it is imperative never to cast explicitly between pointer and non-pointer (which includes ints and oops) otherwise a required translation might not occur. sqMemoryAccess.h defines two macros to replace such casts:

sqInt oopForPointer(char *rawPoitner) and char *pointerForOop(sqInt anOop).

These are effectively no-ops on all combinations other than the "tricky" one described above.

so casting it to an sqInt should be exactly correct.

Except as noted above.

I also assume that firstIndexableField returns a type that is compatible with sqInt.

It returns the address (as a pointer) of the first indexable field. Pointers are always the width of hardware pointers; oops are always integers of the same width as an object pointer as stored in the virtual image. The two are orthogonal. The only way to convert between the two forms is via the two macros described above. Anything else will break on some combination of 32-/64- image and 32-/64- hardware.

Sorry this is turning out to be so confusing! (I was going to rename sqInt to sqOop or something similar, which I think would have lessened the confusion, while I still had the "Interpreter/CCGen token", but got distracted by other things...)

Cheers, Ian

...

...

...

...

On Fri, 29 Apr 2005 09:28:15 -0700, Ian Piumarta ian.piumarta@inria.fr said:

sqMemoryAccess.h defines two macros to replace such casts: sqInt oopForPointer(char *rawPoitner) char *pointerForOop(sqInt anOop). These are effectively no-ops on all combinations other than the

...

...

On Apr 29, 2005, at 09:08, Dave Mason wrote:

...

I also assume that firstIndexableField returns a type that is compatible with sqInt.

OK, thanks Ian, I think I understand now. So Tim's original code:

argPtr = ((float *) (interpreterProxy->firstIndexableField(arg))); for (i = 0; i <= (length - 1); i += 1) { if ((longAt(argPtr + i)) == 0) { return interpreterProxy->primitiveFail(); }

should really be a lot more like:

{sqInt argOop = oopForPointer(interpreterProxy->firstIndexableField(arg)); for (i = 0; i <= (length - 1); i += 1) { if ((floatAt(argOop + i)) == 0.0) { return interpreterProxy->primitiveFail(); } }

although, as you say, intAt might be better, depending on context (but, in general, longAt is just wrong, unless this is actually an array of doubles).

The reason I suggest converting to sqInt rather than leaving everything as pointers is because I assume the indexableFields are allocated on sqInt boundaries, so argOop+i will reference the ith field, while argPtr+i will reference the ith float, which, on a 64-bit machine won't be the same if the fields are on sqInt boundaries. This would be a pretty subtle bug, in that arrays that had the zeros at the end might not call primitiveFail as expected.

(This paragraph is really just to Ian, as I try to come up to speed on some of this, and I'm sure the code is actually correct.) I really need to download the code and look at it, but failing that, I obviously mis-understand something because if the macros are no-ops then the arithmetic above looks wrong. argOop+i will be an integer operation, which is fine for objects of length 1, but not for objects of length sizeof(sqInt). The only way I can see this working is if sqInt isn't actually an int but an int* or long* (depending on architecture) and all the macros do is offset them to/from the vm image's memory pool.

../Dave

On Apr 29, 2005, at 12:09, Dave Mason wrote:

should really be a lot more like:

{sqInt argOop = oopForPointer(interpreterProxy->firstIndexableField(arg)); for (i = 0; i <= (length - 1); i += 1) { if ((floatAt(argOop + i)) == 0.0) { return interpreterProxy->primitiveFail(); } }

I think argPtr should be left as a float *:

1) it's not really beneficial to convert the answer from 1stIndexableField to an oop and then back to a pointer each time the array is accessed (IOW, floatAtPointer() with a raw pointer would be better than floatAt() with an oop);

2) floats are of the right size already, so the pointer arithmetic will scale the index properly (in the code above it would fail since argOop + 1 doesn't point to anything useful);

3) the floats will all be minimally aligned on multiple of their widths, so there isn't any reason to worry about correcting for alignment -- just let the pointer arithmetic do the right thing (assuming whatever creates the float array isn't screwing up on 64-bit machines by aligning on twice their width)

For those reasons I think I'll stand by the (untested, improvised, hand-waving) code I posted earlier. ;)

Cheers, Ian