[Vm-dev] Primitive 40 (asFloat) fails for me
Eliot Miranda
eliot.miranda at gmail.com
Sun Feb 24 20:52:20 UTC 2019
On Sun, Feb 24, 2019 at 12:16 PM Nicolas Cellier <
nicolas.cellier.aka.nice at gmail.com> wrote:
>
>
> 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)
>
I'll rewrite to use a union.
>
> 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
>>
>
--
_,,,^..^,,,_
best, Eliot
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.squeakfoundation.org/pipermail/vm-dev/attachments/20190224/ba0a7c24/attachment.html>
More information about the Vm-dev
mailing list