[Vm-dev] VM Maker: VMMaker.oscog-EstebanLorenzano.1714.mcz

[Nicolas Cellier]

2016-03-09 8:30 GMT+01:00 Esteban Lorenzano <estebanlm at gmail.com>:

>
>
> On 08 Mar 2016, at 22:50, Nicolas Cellier <
> nicolas.cellier.aka.nice at gmail.com> wrote:
>
>
>
> 2016-03-08 22:10 GMT+01:00 Esteban Lorenzano <estebanlm at gmail.com>:
>
>>
>>
>> On 08 Mar 2016, at 21:38, Nicolas Cellier <
>> nicolas.cellier.aka.nice at gmail.com> wrote:
>>
>> IMO the right fix would be to force coercion here:
>>
>>
>> ifTrue:[interpreterProxy signed64BitIntegerFor: (self cCoerceSimple:
>> value to: #sqLong)]
>>
>>
> mmm… kind of disagree, but ok, you know a lot more than me in this issues
> :)
> nevertheless just coercing keeps failing :(
> what actually worked was to use *machine* methods (btw now I understand
> them)… I’m committing now.
>
> Esteban
>
>
Doh, the only thing I know is that it's not going to work, until we
demonstrate it effectively works!
You found the bug, not me, and you see that my proposal doesn't work better
than yours.
Continue experimenting and I'm pretty sure that the little advantage given
by age will soon vanish at this pace :)

Don't forget that the code has to work for extracting 64bits integer on
32bits VM, so a 64bits branch must still exist.
The right thing to do is to first write expectations (tests).

Then, IMO, this piece of code is much too clever, and that's why 3 of us
(Eliot, you, me) did emit false conjectures...
A dumb switch case would be much more robust at the price of a few code
duplications...


>> why?
>>
>
> I recommend writing simple programs like below, compile and run:
>
> #include <stdio.h>
> int main() {
>     int x=-1;
>     unsigned long long y;
>    y = (signed) x;
>    printf(" y <- (signed) x :%llx\n", y);
>    y = (unsigned) x;
>    printf(" y <- (unsigned) x :%llx\n", y);
> }
>
>
>> forcing a coercion to "unsigned int” while we always use sqTypes didn’t
>> feel correct… also long32At will always answer an sqLong, that’s why I
>> thought simpler approach was to remove a coercion instead adding two…
>>
>>
> I wouldn't trust it too much. What is long32At doing exactly?
> I can see two versions of it in platforms/Cross/vm/sqMemoryAccess.h ...
>
> #define long32At    intAt
> # define intAt(oop)                    intAtPointer(atPointerArg(oop))
> # define intAtPointer(ptr)            ((sqInt)(*((int *)(ptr))))
>
> In this case, there will be sign promotion and (ref integerAt: 1 size: 4
> signed: false) will be broken on 64bits VM...
>
> But with the other #if #else branch:
>
> static inline sqInt intAt(sqInt oop)                          { return
> intAtPointer(pointerForOop(oop)); }
> static inline sqInt intAtPointer(char *ptr)            { return
> (sqInt)(*((unsigned int *)ptr)); }
>
> there won't be any sign promotion, and this time the problem will be with
> (ref integerAt: 1 size: 4 signed: true)on 64 bits VM
>
> So the 32 high bits of long32 SHOULD be UNTRUSTED and that's why we need
> to force (int) or (unsigned int) cast in 64 bits VM.
> It's a mistake to have long32 returning a sqInt, it should return a 32bits
> int.
> Currently, int would fit for supported platform, but to be future proof,
> it should have been int32_t
>
> In my own VM brand I have defined unsignedLong32At to be sure to avoid
> these nasty horrible things...
> Most of the time handling unsigned is the right thing to do, they have
> well defined, least surprising behavior.
>
>
>> But maybe we can avoid signed64/unsigned64 and use signedMachineInteger
>> and positiveMachineInteger when byteSize = bytePerWord...
>>
>>
>> that, I have no idea, maybe :)
>>
>
> It's just an optimization for avoiding unecessary 64bits ops.
>
>
>>
>> Esteban
>>
>>
>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://lists.squeakfoundation.org/pipermail/vm-dev/attachments/20160309/2e12e6ce/attachment.htm