cmd/compile: Illegal instruction 'fucomip' on 386 with GO386=387

[Partmedia]

I'm running Go 1.5 on FreeBSD i386 using a "AMD-K6(tm) 3D processor (350.02-MHz 586-class CPU)". I cross-compiled a binary on a modern machine with GOARCH=386 and GO386=387, but the resulting binary crashes:

Program received signal SIGILL, Illegal instruction.
0x0807a395 in runtime.check () at /usr/local/go/src/runtime/runtime1.go:259

Disassembling the problematic instruction reveals:

(gdb) disassemble 0x0807a395 0x0807a396
Dump of assembler code from 0x807a395 to 0x807a396:
0x0807a395 <runtime.check+821>: fucomip %st(1),%st
End of assembler dump.

The documentation advertises:

GO386=387: use x87 for floating point operations; should support all x86 chips (Pentium MMX or later).

MMX is supported, as indicated by the CPU features string:

CPU: AMD-K6(tm) 3D processor (350.02-MHz 586-class CPU)
  Origin="AuthenticAMD"  Id=0x58c  Family=0x5  Model=0x8  Stepping=12
  Features=0x8021bf<FPU,VME,DE,PSE,TSC,MSR,MCE,CX8,PGE,MMX>
  AMD Features=0x80000800<SYSCALL,3DNow!>

This contradicts the documentation; either the documentation should indicate that not all processors with MMX are supported, or the 'fucomip' instruction (and friends) should not be emitted when GO386=387 is set.

[bradfitz]

"The FCOMI/FCOMIP/FUCOMI/FUCOMIP instructions were introduced to the IA-32 Architecture in the P6 family processors and are not available in earlier IA-32 processors."

And the code in ggen.go is using it inside an if gc.Thearch.Use387 { block.

/cc @randall77 @minux @rsc

[randall77]

Yes, it looks like fucomip and friends are Pentium Pro, not MMX.

We should use the non-i versions which are MMX. (The i versions put the comparison results in the eflags register, the non-i ones put the comparison results in the FP status word.)

go 1.4 uses the correct instructions. go 1.5 and 1.6 do not.

Looks like Josh's CL 8738 introduced these new ops. I think the fix should be easy.

@josharian

This is a candidate for 1.5.3.

[rsc]

Not a candidate for 1.5.3.
1.5.3 is a security-only release.
It can be a candidate for 1.5.4 but I don't know that there will be one.

On Tue, Jan 12, 2016 at 2:57 PM, Keith Randall notifications@github.com
wrote:

Yes, it looks like fucomip and friends are Pentium Pro, not MMX.

We should use the non-i versions which are MMX. (The i versions put the
comparison results in the eflags register, the non-i ones put the
comparison results in the FP status word.)

go 1.4 uses the correct instructions. go 1.5 and 1.6 do not.

Looks like Josh's CL 8738 introduced these new ops. I think the fix should
be easy.

@josharian https://github.com/josharian

This is a candidate for 1.5.3.

—
Reply to this email directly or view it on GitHub
#13923 (comment).

[gopherbot]

CL https://golang.org/cl/18590 mentions this issue.

[rsc]

It turns out we've been generating FUCOMIP since Go 1.1. We'll fix this for Go 1.6 but because it's such a long-standing bug we likely will not issue the fix in a Go 1.5 point release.

[Partmedia]

Thanks for the quick response on this issue. I knew that Go binaries (since at least 1.4) always crashed on this machine but didn't investigate until I wanted to run my own Go binaries. I don't imagine many others are running into this issue, so 1.6 is fine.

I'd like to know when this fix becomes available in the tree so I can get running in the meantime.

[randall77]

The fix has been submitted. Please try it out on your processor.

[Partmedia]

It seems like it made it past the runtime check but ended up crashing again:

Program terminated with signal 4, Illegal instruction.
#0  runtime.fastrand1 ()
    at /home/kevinz/workspace/go-freebsd-amd64-bootstrap/src/runtime/asm_386.s:1561

The illegal instruction in question is CMOVS:

(gdb) disassemble
Dump of assembler code for function runtime.fastrand1:
0x080961d0 <runtime.fastrand1+0>:   mov    %gs:0xfffffffc,%eax
0x080961d7 <runtime.fastrand1+7>:   mov    0x18(%eax),%eax
0x080961da <runtime.fastrand1+10>:  mov    0x9c(%eax),%edx
0x080961e0 <runtime.fastrand1+16>:  add    %edx,%edx
0x080961e2 <runtime.fastrand1+18>:  mov    %edx,%ebx
0x080961e4 <runtime.fastrand1+20>:  xor    $0x88888eef,%edx
0x080961ea <runtime.fastrand1+26>:  cmovs  %ebx,%edx
0x080961ed <runtime.fastrand1+29>:  mov    %edx,0x9c(%eax)
0x080961f3 <runtime.fastrand1+35>:  mov    %edx,0x4(%esp)
0x080961f7 <runtime.fastrand1+39>:  ret    
0x080961f8 <runtime.fastrand1+40>:  int3   
0x080961f9 <runtime.fastrand1+41>:  int3   
0x080961fa <runtime.fastrand1+42>:  int3   
0x080961fb <runtime.fastrand1+43>:  int3   
0x080961fc <runtime.fastrand1+44>:  int3   
0x080961fd <runtime.fastrand1+45>:  int3   
0x080961fe <runtime.fastrand1+46>:  int3   
0x080961ff <runtime.fastrand1+47>:  int3   
End of assembler dump.

From asm_386.s:

TEXT runtime·fastrand1(SB), NOSPLIT, $0-4
    get_tls(CX)
    MOVL    g(CX), AX
    MOVL    g_m(AX), AX
    MOVL    m_fastrand(AX), DX
    ADDL    DX, DX
    MOVL    DX, BX
    XORL    $0x88888eef, DX
    CMOVLMI BX, DX
    MOVL    DX, m_fastrand(AX)
    MOVL    DX, ret+0(FP)
    RET

It seems like the 'fucomip' issue was fixed but I'm not sure because 'cmovs' showed up.

[randall77]

cmov fix out for review. This one is my fault from a ways back. maps, selects, string ==, all used cmov. How did this ever work?

Hopefully there won't be too many rounds of this.

We really need a Pentium MMX builder so we don't regress yet again.