[Vm-dev] Primitive 40 (asFloat) fails for me
Eliot Miranda
eliot.miranda at gmail.com
Fri Feb 22 20:02:58 UTC 2019
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.
> Levente
>
> On Fri, 22 Feb 2019, Tobias Pape wrote:
>
> >
> > Hi Levente.
> >
> >> On 22.02.2019, at 00:43, Levente Uzonyi <leves at caesar.elte.hu> wrote:
> >>
> >> I had a look at the generated code and I think the generated code for
> primitiveAsFloat has no issues. The machine code is simple and looks
> correct with gcc 8, thought the pxor operation is unnecessary (and not
> generated by gcc 4) as cvtsi2sd will zero fill the higher 64 bits of xmm0.
> >>
> >> GCC 4.8:
> >>
> >> 0000000000432ff0 <primitiveAsFloat>:
> >> { DECL_MAYBE_SQ_GLOBAL_STRUCT
> >> 432ff0: 53 push %rbx
> >> rcvr = longAt(GIV(stackPointer));
> >> 432ff1: 48 8b 1d 20 24 36 00 mov 0x362420(%rip),%rbx
> # 795418 <stackPointer>
> >> longAtput((sp = GIV(stackPointer) + ((1 - 1) * BytesPerWord)),
> floatObjectOf(((double) ((rcvr >> 3)) )));
> >> 432ff8: 48 8b 03 mov (%rbx),%rax
> >> 432ffb: 48 c1 f8 03 sar $0x3,%rax
> >> 432fff: f2 48 0f 2a c0 cvtsi2sd %rax,%xmm0
> >> 433004: e8 37 ee ff ff callq 431e40 <floatObjectOf>
> >> 433009: 48 89 03 mov %rax,(%rbx)
> >> GIV(stackPointer) = sp;
> >> 43300c: 48 89 1d 05 24 36 00 mov %rbx,0x362405(%rip)
> # 795418 <stackPointer>
> >> }
> >> 433013: 5b pop %rbx
> >> 433014: c3 retq
> >> 433015: 90 nop
> >> 433016: 66 2e 0f 1f 84 00 00 nopw %cs:0x0(%rax,%rax,1)
> >> 43301d: 00 00 00
> >>
> >> GCC 8.2:
> >>
> >> 000000000004a1f0 <primitiveAsFloat>:
> >> { DECL_MAYBE_SQ_GLOBAL_STRUCT
> >> 4a1f0: 53 push %rbx
> >> rcvr = longAt(GIV(stackPointer));
> >> 4a1f1: 48 8b 1d 80 32 36 00 mov 0x363280(%rip),%rbx
> # 3ad478 <stackPointer>
> >> longAtput((sp = GIV(stackPointer) + ((1 - 1) * BytesPerWord)),
> floatObjectOf(((double) ((rcvr >> 3)) )));
> >> 4a1f8: 66 0f ef c0 pxor %xmm0,%xmm0
> >> 4a1fc: 48 8b 03 mov (%rbx),%rax
> >> 4a1ff: 48 c1 f8 03 sar $0x3,%rax
> >> 4a203: f2 48 0f 2a c0 cvtsi2sd %rax,%xmm0
> >> 4a208: e8 b3 eb ff ff callq 48dc0 <floatObjectOf>
> >> 4a20d: 48 89 03 mov %rax,(%rbx)
> >> GIV(stackPointer) = sp;
> >> 4a210: 48 89 1d 61 32 36 00 mov %rbx,0x363261(%rip)
> # 3ad478 <stackPointer>
> >> }
> >> 4a217: 5b pop %rbx
> >> 4a218: c3 retq
> >> 4a219: 0f 1f 80 00 00 00 00 nopl 0x0(%rax)
> >>
> >>
> >> Actually the returned float object is -100.0 (-100 asFloat < 0.0
> returns true), but the code used for printing doesn't work properly,
> because #basicAt: returns 0 (when the argument is 1 or 2), and the method
> used to print the number uses #signBit to print the negative sign, and
> #signBit relies on #basicAt: returning a non-zero value when the number is
> negative.
> >>
> >> So, we're back to the original issue I found: #basicAt: (primitive 38)
> doesn't work with newer gcc versions.
> >>
> >
> > Wow. Great analysis! Thank you :)
> >
> > Best regards
> > -Tobias
>
--
_,,,^..^,,,_
best, Eliot
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.squeakfoundation.org/pipermail/vm-dev/attachments/20190222/5476b645/attachment.html>
More information about the Vm-dev
mailing list