[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