Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

cmd/compile: plan9-amd64-9front builder failing in TestFma #35063

Closed
fhs opened this issue Oct 22, 2019 · 12 comments
Closed

cmd/compile: plan9-amd64-9front builder failing in TestFma #35063

fhs opened this issue Oct 22, 2019 · 12 comments

Comments

@fhs
Copy link
Contributor

@fhs fhs commented Oct 22, 2019

Since CL 137156
(https://build.golang.org/log/9c5e9a06aa97d909dc1ef2e2edca437e7a320e7e) the builder has been failing in TestFma in math package:

fatal error: sys: trap: #UD pc=0x308886
[signal sys: trap: code=0x0 addr=0x0 pc=0x308886]

goroutine 67 [running]:
runtime.throw(0x4012000, 0x1a)
	/tmp/workdir-gnot/go/src/runtime/panic.go:774 +0x81 fp=0x402ee10 sp=0x402ede0 pc=0x22d3c1
runtime.sigpanic()
	/tmp/workdir-gnot/go/src/runtime/os_plan9.go:104 +0x367 fp=0x402ee80 sp=0x402ee10 pc=0x229c57
math_test.TestFma(0x4095d00)
	/tmp/workdir-gnot/go/src/math/all_test.go:3058 +0x96 fp=0x402ef70 sp=0x402ee80 pc=0x308886
testing.tRunner(0x4095d00, 0x360698)
	/tmp/workdir-gnot/go/src/testing/testing.go:909 +0xce fp=0x402efd0 sp=0x402ef70 pc=0x2babde
runtime.goexit()
	/tmp/workdir-gnot/go/src/runtime/asm_amd64.s:1375 +0x1 fp=0x402efd8 sp=0x402efd0 pc=0x257431
created by testing.(*T).Run
	/tmp/workdir-gnot/go/src/testing/testing.go:960 +0x363

goroutine 1 [chan receive]:
testing.(*T).Run(0x4095d00, 0x357374, 0x7, 0x360698, 0x26b201)
	/tmp/workdir-gnot/go/src/testing/testing.go:961 +0x38a
testing.runTests.func1(0x4084000)
	/tmp/workdir-gnot/go/src/testing/testing.go:1202 +0x7d
testing.tRunner(0x4084000, 0x4030db0)
	/tmp/workdir-gnot/go/src/testing/testing.go:909 +0xce
testing.runTests(0x4016060, 0x61c720, 0x47, 0x47, 0x0)
	/tmp/workdir-gnot/go/src/testing/testing.go:1200 +0x2c9
testing.(*M).Run(0x4080000, 0x0)
	/tmp/workdir-gnot/go/src/testing/testing.go:1117 +0x17b
main.main()
	_testmain.go:394 +0x13a
FAIL	math	0.441s
FAIL

CC @0intro
@gopherbot please add label OS-Plan9, NeedsInvestigation

@agnivade

This comment has been minimized.

Copy link
Contributor

@agnivade agnivade commented Oct 22, 2019

@agnivade agnivade added this to the Go1.14 milestone Oct 22, 2019
@smasher164 smasher164 self-assigned this Oct 22, 2019
@smasher164

This comment has been minimized.

Copy link
Member

@smasher164 smasher164 commented Oct 22, 2019

A floating-point panic occurs on a call to the Fma function because of the following instruction sequence:

all_test.go:3058    0x3089b6        803d571a330000      CMPB $0x0, runtime.x86HasFMA(SB)    
all_test.go:3058    0x3089bd        0f84d8010000        JE 0x308b9b             
all_test.go:3058    0x3089c3        0f10e2              MOVUPS X2, X4               
all_test.go:3058    0x3089c6        c4e2e1b9d0660f2e    MOVL $0x2e0f66d0, CX            
bits.go:39          0x3089ce        d2                  ?

The comparison of runtime.x86HasFMA with 0 returns false, indicating that the instruction is present. The instruction at 0x3089c6 is reached, where c4e2e1b9d0660f2e and d2 actually decode as

vfmadd231sd %xmm0,%xmm3,%xmm2
ucomisd     %xmm2,%xmm2

The floating-point panic is a result of executing the vfmadd231sd instruction. However, I am not familiar enough with Plan 9 (9front) to know why the instruction doesn't execute. Is amd64 feature detection transparently returning true on the builder even if the instruction is not available? Or is there some restriction present in the 9front kernel? #15234 may be related.

@smasher164 smasher164 removed their assignment Oct 22, 2019
@0intro

This comment has been minimized.

Copy link
Member

@0intro 0intro commented Oct 22, 2019

We usually disable SSE instructions on Plan 9 because FP aren't allowed in note handler. I think we should disable FMA on Plan 9.

@smasher164

This comment has been minimized.

Copy link
Member

@smasher164 smasher164 commented Oct 22, 2019

I think we should disable FMA on Plan 9.

Sounds good to me. I'll send a CL shortly.

@0intro

This comment has been minimized.

Copy link
Member

@0intro 0intro commented Oct 22, 2019

Look at how useSSE is done in src/cmd/compile/internal/ssa. Maybe we can add a useFMA configuration parameter.

@smasher164

This comment has been minimized.

Copy link
Member

@smasher164 smasher164 commented Oct 22, 2019

Given that the x86HasFMA is already set up in src/runtime/proc.go as

x86HasFMA = cpu.X86.HasFMA 

Can't I just change it to

x86HasFMA = cpu.X86.HasFMA && GOOS != "plan9"

This will cause feature detection to fail and default to the software fallback.

@0intro

This comment has been minimized.

Copy link
Member

@0intro 0intro commented Oct 22, 2019

I think it's better to keep the compiler configuration separate from the CPU feature detection. That's what we did for SSE.

@0intro

This comment has been minimized.

Copy link
Member

@0intro 0intro commented Oct 22, 2019

I think you can send a CL with your initial idea.

@smasher164

This comment has been minimized.

Copy link
Member

@smasher164 smasher164 commented Oct 22, 2019

I sent CL https://go-review.googlesource.com/c/go/+/202617 which introduces a UseFMA flag to ssa.Config, and checks it in addF to call the software fallback on AMD64.

@gopherbot

This comment has been minimized.

Copy link

@gopherbot gopherbot commented Oct 22, 2019

Change https://golang.org/cl/202617 mentions this issue: cmd/compile: don't use FMA on plan9/amd64

@smasher164

This comment has been minimized.

Copy link
Member

@smasher164 smasher164 commented Oct 22, 2019

@0intro Is the plan9-amd64-9front builder down? I don't see it on the dashboard, and the trybot has been stuck on wait_static_builder.

@0intro

This comment has been minimized.

Copy link
Member

@0intro 0intro commented Oct 22, 2019

@smasher164 Sorry, the builder is back online.

@gopherbot gopherbot closed this in 03fb1f6 Oct 22, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
5 participants
You can’t perform that action at this time.