[Vm-dev] Primitive 40 (asFloat) fails for me

Nicolas Cellier nicolas.cellier.aka.nice at gmail.com
Sun Feb 24 20:15:46 UTC 2019


Hi Eliot,
see below...

Le ven. 22 févr. 2019 à 21:03, Eliot Miranda <eliot.miranda at gmail.com> a
écrit :

>
> Hi Levente,
>
> On Fri, Feb 22, 2019 at 1:58 AM Levente Uzonyi <leves at caesar.elte.hu>
> wrote:
>
>>
>> Thanks. I spent some time in gdb land and there seems to be a miscompiled
>> conditional jump. The question is: is it a compiler bug or a bug in the
>> Smalltalk -> C translation. With gcc 4.8, the correct jump instruction is
>> generated.
>>
>> Here is the disassembled code with my comments following the path when
>> -100.0 basicAt: 1 is evaluated in the image:
>>
>> Dump of assembler code for function primitiveFloatAt:
>>     0x0000555555589ec0 <+0>:    mov    0x3775b1(%rip),%rdx        #
>> 0x555555901478 <stackPointer>
>> %rax will hold the index (1 or 2) as a SmallInteger (so the actual value
>> will be 0x9 or 0x11 for 1 and 2 respectively).
>>     0x0000555555589ec7 <+7>:    mov    (%rdx),%rax
>> %rcx is the receiver object's address
>>     0x0000555555589eca <+10>:   mov    0x8(%rdx),%rcx
>> Is the index SmallInteger 1?
>>     0x0000555555589ece <+14>:   cmp    $0x9,%rax
>> If yes, jump to 0x555555589ef8
>>     0x0000555555589ed2 <+18>:   je     0x555555589ef8
>> <primitiveFloatAt+56>
>>     0x0000555555589ed4 <+20>:   cmp    $0x11,%rax
>>     0x0000555555589ed8 <+24>:   je     0x555555589f30
>> <primitiveFloatAt+112>
>>     0x0000555555589eda <+26>:   and    $0x7,%eax
>>     0x0000555555589edd <+29>:   cmp    $0x1,%rax
>>     0x0000555555589ee1 <+33>:   sete   %al
>>     0x0000555555589ee4 <+36>:   movzbl %al,%eax
>>     0x0000555555589ee7 <+39>:   add    $0x3,%rax
>>     0x0000555555589eeb <+43>:   mov    %rax,0x37757e(%rip)        #
>> 0x555555901470 <primFailCode>
>>     0x0000555555589ef2 <+50>:   retq
>>     0x0000555555589ef3 <+51>:   nopl   0x0(%rax,%rax,1)
>> Set the value of  %eax to 0 for seemingly no reason yet. The returned
>> SmallInteger will be computed in %rax, so this is important.
>>     0x0000555555589ef8 <+56>:   xor    %eax,%eax
>> Check if the receiver has the SmallFloat tag set (0x4)
>>     0x0000555555589efa <+58>:   test   $0x4,%cl
>> If yes, jump to  0x555555589f03. This is the miscompiled conditional
>> jump. It should go to 0x0000555555589f37, but it will just skip the next
>> instruction leaving %eax set to 0.
>> => 0x0000555555589efd <+61>:    jne    0x555555589f03
>> <primitiveFloatAt+67>
>> Copy the 2nd 32-bit field of the receiver into %rax with sign extension?
>>     0x0000555555589eff <+63>:   movslq 0xc(%rcx),%rax
>> Shift by 3 bits to make room for the tag.
>>     0x0000555555589f03 <+67>:   shl    $0x3,%rax
>> Adjust the stack pointer.
>>     0x0000555555589f07 <+71>:   add    $0x8,%rdx
>> Prepare a 32-bit mask shifted left by 3 bits for the tag.
>>     0x0000555555589f0b <+75>:   movabs $0x7fffffff8,%rcx
>> Mask the result.
>>     0x0000555555589f15 <+85>:   and    %rcx,%rax
>> Add the SmallInteger tag.
>>     0x0000555555589f18 <+88>:   or     $0x1,%rax
>> Store the result on the stack.
>>     0x0000555555589f1c <+92>:   mov    %rax,(%rdx)
>>     0x0000555555589f1f <+95>:   mov    %rdx,0x377552(%rip)        #
>> 0x555555901478 <stackPointer>
>>     0x0000555555589f26 <+102>:  retq
>>     0x0000555555589f27 <+103>:  nopw   0x0(%rax,%rax,1)
>>     0x0000555555589f30 <+112>:  xor    %eax,%eax
>>     0x0000555555589f32 <+114>:  test   $0x4,%cl
>>     0x0000555555589f35 <+117>:  jne    0x555555589f03
>> <primitiveFloatAt+67>
>> Copy the 1st 32-bit field of the receiver into %rax with sign extension?
>>     0x0000555555589f37 <+119>:  movslq 0x8(%rcx),%rax
>>     0x0000555555589f3b <+123>:  jmp    0x555555589f03
>> <primitiveFloatAt+67>
>> End of assembler dump.
>>
>
> I'm pretty sure the C code is correct, but Nicolas is our C lawyer.
> However we need to look at both primitiveFloatAt
> and fetchLong32:ofFloatObject:. Here's the source:
>
>     /* Spur64BitMemoryManager>>#fetchLong32:ofFloatObject: */
> static sqInt NoDbgRegParms
> fetchLong32ofFloatObject(sqInt fieldIndex, sqInt oop)
> {
>     usqLong bits;
>     usqLong rot;
>
>     if (!(oop & (smallFloatTag()))) {
>         return long32At((oop + BaseHeaderSize) +
> (((sqInt)((usqInt)(fieldIndex) << 2))));
>     }
>     /* begin smallFloatBitsOf: */
>     assert(isImmediateFloat(oop));
>     rot = ((usqInt) (((usqInt)oop))) >> (numTagBits());
>     if (rot > 1) {
>
>         /* a.k.a. ~= +/-0.0 */
>         rot += ((sqInt)((usqInt)((smallFloatExponentOffset())) <<
> ((smallFloatMantissaBits()) + 1)));
>     }
>     /* begin rotateRight: */
>     rot = (rot << 0x3F) + (((usqInt) (((usqInt)rot))) >> 1);
>     bits = rot;
>     return (((int *) ((&bits))))[fieldIndex];
> }
>
> Note that [+/-]100.0 is an immediate float.  Nicolas, is this kind of
> pointer punning still OK?  Notice that at no time do we pun the bits to a
> double.  They are always integral bits.  What we're doing is manipulating
> the bit pattern for -100.0, an immediate
> SmallFloat64. (Spur64BitMemoryManager new smallFloatObjectOf: -100.0) hex
> '16r859000000000000C'  where 16r4 is the tag pattern for an immediate float
> and 16r8 is the sign bit in the least significant position.
>

In theory, pointer type aliasing is Undefined Behavior.
Though, I don't well see what kind of optimization a compiler could perform
here...
Try one of the 2 solid choices for type punning:
- memcpy
- union (at least in C it's legal, I'm not even sure for C++ and don't have
time to search now)
or you may try the more fragile -fno-strict-aliasing (or something like
that)

Note that a clever enough compiler should not invoke memcpy at all in
simple cases like this.
(If it is clever enough to misscompile this simple case of pointer
aliasing, I don't see why it wouldn't for memcpy).

-- 
> _,,,^..^,,,_
> best, Eliot
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.squeakfoundation.org/pipermail/vm-dev/attachments/20190224/c2655b0c/attachment.html>


More information about the Vm-dev mailing list