[Vm-dev] Primitive 40 (asFloat) fails for me
Eliot Miranda
eliot.miranda at gmail.com
Sun Feb 24 20:56:24 UTC 2019
On Sun, Feb 24, 2019 at 12:52 PM Eliot Miranda <eliot.miranda at gmail.com>
wrote:
>
>
> 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.
>
Ugh, not yet. That would be so much uglier and m ore complicated. Let's
pin down where the bug is first.
>
>
>>
>> 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).
>>
>
+1.
_,,,^..^,,,_
best, Eliot
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.squeakfoundation.org/pipermail/vm-dev/attachments/20190224/b88379c2/attachment-0001.html>
More information about the Vm-dev
mailing list