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

[Esteban Lorenzano]

> 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 <mailto:estebanlm at gmail.com>>:
>  
> 
>> On 08 Mar 2016, at 21:38, Nicolas Cellier <nicolas.cellier.aka.nice at gmail.com <mailto: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

> 
> 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/3b5128b8/attachment.htm