<div dir="ltr"><div dir="ltr"><br></div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Sun, Feb 24, 2019 at 12:16 PM Nicolas Cellier <<a href="mailto:nicolas.cellier.aka.nice@gmail.com">nicolas.cellier.aka.nice@gmail.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-style:solid;border-left-color:rgb(204,204,204);padding-left:1ex"> <div dir="ltr"><div dir="ltr"><br></div><div>Hi Eliot,</div><div>see below...</div><div><br></div><div><div class="gmail_quote"><div dir="ltr" class="gmail_attr">Le ven. 22 févr. 2019 à 21:03, Eliot Miranda <<a href="mailto:eliot.miranda@gmail.com" target="_blank">eliot.miranda@gmail.com</a>> a écrit :<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-style:solid;border-left-color:rgb(204,204,204);padding-left:1ex"> <div dir="ltr"><div dir="ltr"><div dir="ltr"><div dir="ltr"><div dir="ltr"><div dir="ltr">Hi Levente,</div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Fri, Feb 22, 2019 at 1:58 AM Levente Uzonyi <<a href="mailto:leves@caesar.elte.hu" target="_blank">leves@caesar.elte.hu</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-style:solid;border-left-color:rgb(204,204,204);padding-left:1ex"> <br>
Thanks. I spent some time in gdb land and there seems to be a miscompiled <br>
conditional jump. The question is: is it a compiler bug or a bug in the <br>
Smalltalk -> C translation. With gcc 4.8, the correct jump instruction is <br>
generated.<br>
<br>
Here is the disassembled code with my comments following the path when <br>
-100.0 basicAt: 1 is evaluated in the image:<br>
<br>
Dump of assembler code for function primitiveFloatAt:<br>
0x0000555555589ec0 <+0>: mov 0x3775b1(%rip),%rdx # 0x555555901478 <stackPointer><br>
%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).<br>
0x0000555555589ec7 <+7>: mov (%rdx),%rax<br>
%rcx is the receiver object's address<br>
0x0000555555589eca <+10>: mov 0x8(%rdx),%rcx<br>
Is the index SmallInteger 1?<br>
0x0000555555589ece <+14>: cmp $0x9,%rax<br>
If yes, jump to 0x555555589ef8<br>
0x0000555555589ed2 <+18>: je 0x555555589ef8 <primitiveFloatAt+56><br>
0x0000555555589ed4 <+20>: cmp $0x11,%rax<br>
0x0000555555589ed8 <+24>: je 0x555555589f30 <primitiveFloatAt+112><br>
0x0000555555589eda <+26>: and $0x7,%eax<br>
0x0000555555589edd <+29>: cmp $0x1,%rax<br>
0x0000555555589ee1 <+33>: sete %al<br>
0x0000555555589ee4 <+36>: movzbl %al,%eax<br>
0x0000555555589ee7 <+39>: add $0x3,%rax<br>
0x0000555555589eeb <+43>: mov %rax,0x37757e(%rip) # 0x555555901470 <primFailCode><br>
0x0000555555589ef2 <+50>: retq<br>
0x0000555555589ef3 <+51>: nopl 0x0(%rax,%rax,1)<br>
Set the value of %eax to 0 for seemingly no reason yet. The returned SmallInteger will be computed in %rax, so this is important.<br>
0x0000555555589ef8 <+56>: xor %eax,%eax<br>
Check if the receiver has the SmallFloat tag set (0x4)<br>
0x0000555555589efa <+58>: test $0x4,%cl<br>
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.<br>
=> 0x0000555555589efd <+61>: jne 0x555555589f03 <primitiveFloatAt+67> <br>
Copy the 2nd 32-bit field of the receiver into %rax with sign extension?<br>
0x0000555555589eff <+63>: movslq 0xc(%rcx),%rax<br>
Shift by 3 bits to make room for the tag.<br>
0x0000555555589f03 <+67>: shl $0x3,%rax<br>
Adjust the stack pointer.<br>
0x0000555555589f07 <+71>: add $0x8,%rdx<br>
Prepare a 32-bit mask shifted left by 3 bits for the tag.<br>
0x0000555555589f0b <+75>: movabs $0x7fffffff8,%rcx<br>
Mask the result.<br>
0x0000555555589f15 <+85>: and %rcx,%rax<br>
Add the SmallInteger tag.<br>
0x0000555555589f18 <+88>: or $0x1,%rax<br>
Store the result on the stack.<br>
0x0000555555589f1c <+92>: mov %rax,(%rdx)<br>
0x0000555555589f1f <+95>: mov %rdx,0x377552(%rip) # 0x555555901478 <stackPointer><br>
0x0000555555589f26 <+102>: retq<br>
0x0000555555589f27 <+103>: nopw 0x0(%rax,%rax,1)<br>
0x0000555555589f30 <+112>: xor %eax,%eax<br>
0x0000555555589f32 <+114>: test $0x4,%cl<br>
0x0000555555589f35 <+117>: jne 0x555555589f03 <primitiveFloatAt+67><br>
Copy the 1st 32-bit field of the receiver into %rax with sign extension?<br>
0x0000555555589f37 <+119>: movslq 0x8(%rcx),%rax<br>
0x0000555555589f3b <+123>: jmp 0x555555589f03 <primitiveFloatAt+67><br>
End of assembler dump.<br></blockquote><div><br></div><div>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:</div><div><br></div><div><div> /* Spur64BitMemoryManager>>#fetchLong32:ofFloatObject: */</div><div>static sqInt NoDbgRegParms</div><div>fetchLong32ofFloatObject(sqInt fieldIndex, sqInt oop)</div><div>{</div><div> usqLong bits;</div><div> usqLong rot;</div><div><br></div><div> if (!(oop & (smallFloatTag()))) {</div><div> return long32At((oop + BaseHeaderSize) + (((sqInt)((usqInt)(fieldIndex) << 2))));</div><div> }</div><div> /* begin smallFloatBitsOf: */</div><div> assert(isImmediateFloat(oop));</div><div> rot = ((usqInt) (((usqInt)oop))) >> (numTagBits());</div><div> if (rot > 1) {</div><div><br></div><div> /* a.k.a. ~= +/-0.0 */</div><div> rot += ((sqInt)((usqInt)((smallFloatExponentOffset())) << ((smallFloatMantissaBits()) + 1)));</div><div> }</div><div> /* begin rotateRight: */</div><div> rot = (rot << 0x3F) + (((usqInt) (((usqInt)rot))) >> 1);</div><div> bits = rot;</div><div> return (((int *) ((&bits))))[fieldIndex];</div><div>}</div></div><div><br></div><div>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.</div></div></div></div></div></div></div></blockquote><div><br></div><div>In theory, pointer type aliasing is Undefined Behavior.</div><div>Though, I don't well see what kind of optimization a compiler could perform here...<br></div><div>Try one of the 2 solid choices for type punning:</div><div>- memcpy</div><div>- union (at least in C it's legal, I'm not even sure for C++ and don't have time to search now)<br></div><div>or you may try the more fragile -fno-strict-aliasing (or something like that)<br></div></div></div></div></blockquote><div><br></div><div>I'll rewrite to use a union.</div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-style:solid;border-left-color:rgb(204,204,204);padding-left:1ex"><div dir="ltr"><div><div class="gmail_quote"><div></div><div><br></div><div>Note that a clever enough compiler should not invoke memcpy at all in simple cases like this.</div><div>(If it is clever enough to misscompile this simple case of pointer aliasing, I don't see why it wouldn't for memcpy).<br></div><div><br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-style:solid;border-left-color:rgb(204,204,204);padding-left:1ex"><div dir="ltr"><div dir="ltr"><div dir="ltr"><div dir="ltr"><div dir="ltr"><div class="gmail_quote">-- <br></div><div dir="ltr" class="gmail-m_7234226069848986307gmail-m_-5487661887483613610gmail_signature"><div dir="ltr"><div><span style="font-size:small;border-collapse:separate"><div>_,,,^..^,,,_<br></div><div>best, Eliot</div></span></div></div></div></div></div></div></div></div>
</blockquote></div></div></div>
</blockquote></div><br clear="all"><div><br></div>-- <br><div dir="ltr" class="gmail_signature"><div dir="ltr"><div><span style="font-size:small;border-collapse:separate"><div>_,,,^..^,,,_<br></div><div>best, Eliot</div></span></div></div></div></div>