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: unaligned float64 load/store on mips #18140

Closed
CetinSert opened this issue Dec 1, 2016 · 39 comments
Closed

cmd/compile: unaligned float64 load/store on mips #18140

CetinSert opened this issue Dec 1, 2016 · 39 comments
Milestone

Comments

@CetinSert
Copy link

CetinSert commented Dec 1, 2016

What version of Go are you using (go version)?

go version go1.8beta1 windows/amd64

What operating system and processor architecture are you using (go env)?

Building on windows-amd64 for linux-mips.

set GOARCH=mips
set GOBIN=
set GOEXE=
set GOHOSTARCH=amd64
set GOHOSTOS=windows
set GOOS=linux
set GOPATH=C:\Users\cetin\go
set GORACE=
set GOROOT=C:\Go
set GOTOOLDIR=C:\Go\pkg\tool\windows_amd64
set GCCGO=gccgo
set CC=gcc
set GOGCCFLAGS=-fPIC -fmessage-length=0
set CXX=g++
set CGO_ENABLED=0
set PKG_CONFIG=pkg-config
set CGO_CFLAGS=-g -O2
set CGO_CPPFLAGS=
set CGO_CXXFLAGS=-g -O2
set CGO_FFLAGS=-g -O2
set CGO_LDFLAGS=-g -O2

What did you do?

Windows PC (cmd.exe):

set GOOS=linux&&set GOARCH=mips&&go build

MIPS Router:

./hello
Illegal instruction

What did you expect to see?

Hello world!

What did you see instead?

Illegal instruction

What kind of hardware products does the recent mips support merge target then?

@CetinSert CetinSert changed the title Illegal instruction on mips router: GL-AR300M Illegal instruction on mips router: GL-AR300M and GL-AR150 Dec 1, 2016
@vstefanovic
Copy link
Member

The supported instruction set is mips32(r1). Illegal instruction you're seeing is likely due to the kernel not handling unaligned loads and stores. Fixing go compiler to not emit such loads and stores is currently in the works and thanks to this issue we could treat it as a fix instead of new feature - so hopefully it can be included in the 1.8 release.

@bradfitz
Copy link
Contributor

bradfitz commented Dec 1, 2016

Looks like those devices are:

https://wikidevi.com/wiki/MIPS_24K and
https://wikidevi.com/wiki/MIPS_74K

@vstefanovic, it sounds like you expect those to work?

How much work and how invasive is it? If it only affects mips and nothing else, it might still be acceptable for Go 1.8.

But it would need to be soon regardless.

@bradfitz
Copy link
Contributor

bradfitz commented Dec 1, 2016

/cc @ianlancetaylor @cherrymui

@bradfitz bradfitz changed the title Illegal instruction on mips router: GL-AR300M and GL-AR150 cmd/compile: mips: Illegal instruction on mips router: GL-AR300M and GL-AR150 Dec 1, 2016
@bradfitz bradfitz added this to the Go1.8Maybe milestone Dec 1, 2016
@quentinmit quentinmit added the NeedsFix The path to resolution is known, but the work has not been done. label Dec 1, 2016
@cherrymui
Copy link
Member

I don't think the compiler generate any unaligned loads/stores. @vstefanovic, does it?
The only thing I see is the assembly functions in the runtime that have MOVWL, MOVWR.

@pqyptixa
Copy link

pqyptixa commented Dec 2, 2016

I was seeing the same problem ("Illegal instruction") in a TP-Link TL-WR1043ND router running OpenWRT, and after attaching a debugger, I noticed that the problem was that the CPU lacks support for floating point instruction.
Perhaps the OP has the same problem? Also, related: #17984
You'll probably get many similar bug reports in the next month(s) :)

@minux
Copy link
Member

minux commented Dec 2, 2016 via email

@bradfitz
Copy link
Contributor

bradfitz commented Dec 2, 2016

@minux, yes. Want to send a CL? Or @cherrymui?

Also, https://github.com/golang/go/wiki/MinimumRequirements needs to be updated.

@bradfitz bradfitz modified the milestones: Go1.8, Go1.8Maybe Dec 2, 2016
@bradfitz bradfitz changed the title cmd/compile: mips: Illegal instruction on mips router: GL-AR300M and GL-AR150 cmd/compile: mips: document requirements, Illegal instruction on mips router: GL-AR300M and GL-AR150 Dec 2, 2016
@minux
Copy link
Member

minux commented Dec 2, 2016 via email

@gopherbot
Copy link

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

@CetinSert
Copy link
Author

CetinSert commented Dec 2, 2016

How can I check if an already compiled and flashed OpenWRT has kernel FPU emulation enabled or not?

Also for GL-AR150, we are using self-compiled images which in the default settings of OpenWRT's make menuconfig suggest FPU emulation is enabled.

If anyone needs online access to any of the devices, I can set them up a permanent tunnel for as long as they need.


Which consumer router meets the requirements of the MIPS 32 support the way it is in Beta 1?

As NeedsFix still stands, ist it right to assume support will be extended to cover devices mentioned under this issue by me and others?

@vstefanovic
Copy link
Member

@cherrymui
There are non-8-byte aligned ldc1's and sdc1's for accessing doubles on the stack because $sp is often not 8-byte aligned - the first reason for that is reserving only 4 bytes for $ra on the stack.

@bradfitz
I expect the issue with these two CPUs is kernel not handling unaligned loads and stores.
The patch for compiler not emitting them is not invasive or large, it has a few "if arch == 'mips'" in the common code. It should be ready during the next week.

@CetinSert
If this c code doesn't raise illegal instruction, then your machine can stand FP instructions:

int main(int argc, char** argh) {
asm("add.d $f0,$f0,$f0");
return 0;
}

And this one should tell you if the kernel handles unaligned loads and stores:

int main(int argc, char** argh) {
asm("ldc1 $f0,4($sp)");
return 0;
}

Btw, mips soft float patch is almost ready.

@cherrymui
Copy link
Member

There are non-8-byte aligned ldc1's and sdc1's

Those probably shouldn't be there. Maybe we should make the assembler rewrite MOVD x, F0 to something like MOVF x, F0; MOVF x+4, F1?

@vstefanovic
Copy link
Member

The proposed change skips 4 bytes after $ra and places args starting from $sp +8. Then we could keep using ldc1 instead of two lwc1.

@minux
Copy link
Member

minux commented Dec 3, 2016 via email

@vstefanovic
Copy link
Member

These are aligned with
t.Align = uint8(2 * Widthreg)
in case of mips and float64 (in align.go:dowidth()).

Yes, the softfloat code is using runtime/softfloat64.go.

@minux
Copy link
Member

minux commented Dec 6, 2016 via email

@bradfitz
Copy link
Contributor

bradfitz commented Dec 7, 2016

@vstefanovic, that's a fine comment as an FYI, but we don't track closed issues, so be sure there's an open bug somewhere actually tracking those changes.

@CetinSert
Copy link
Author

@vstefanovic I appreciate continued comments under this issue and would love to learn where to track further development in case you follow the advice of @bradfitz .

@minux
Copy link
Member

minux commented Dec 7, 2016 via email

@minux minux changed the title cmd/compile: mips: document requirements, Illegal instruction on mips router: GL-AR300M and GL-AR150 cmd/compile: unaligned float64 load/store on mips Dec 7, 2016
@minux minux removed Documentation NeedsFix The path to resolution is known, but the work has not been done. labels Dec 7, 2016
@minux minux reopened this Dec 7, 2016
@minux
Copy link
Member

minux commented Dec 7, 2016 via email

@CetinSert
Copy link
Author

For all, why / where does a Hello World binary even touch any floats before it gets to execute any user code (fmt.Println("Hello World!"))?

@minux
Copy link
Member

minux commented Dec 7, 2016 via email

@bradfitz
Copy link
Contributor

bradfitz commented Dec 8, 2016

@randall77, @ianlancetaylor ... thoughts on this? As mips is a new architecture in Go 1.8, what's acceptable to still get in for Go 1.8?

@vstefanovic
Copy link
Member

Replacing a ldc1 with two lwc1 would be more than ok for 1.8, especially if we'll be 8-byte aligning arm and 386 too in 1.9. Then we can align them all at the same time.

@minux
Copy link
Member

minux commented Dec 9, 2016 via email

@gopherbot
Copy link

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

@l2dy
Copy link

l2dy commented Dec 18, 2016

I'm building with go version go1.8beta2 darwin/amd64 for linux/mipsle and still got Illegal instruction on my MIPS router (Lenovo Y1, OpenWrt with musl).

@minux
Copy link
Member

minux commented Dec 18, 2016 via email

@icexin
Copy link

icexin commented Dec 22, 2016

I have the same issue on wndr4300. After kernel FPU emulation is enabled, the binary can run normally. Hoping to help you.

@CetinSert
Copy link
Author

@icexin how did you enable that?

@icexin
Copy link

icexin commented Dec 23, 2016

@CetinSert Change directory to openwrt build dir, make kernel_menuconfigKernel type ---> MIPS FPU Emulator

@CetinSert
Copy link
Author

@icexin that indeed did the trick! I was not even aware there was a kernel_menuconfig I had beenlooking for that option under menuconfig o__O"

@pqyptixa
Copy link

@vstefanovic : you say the "mips soft float patch is almost ready". I wonder, will this patch be merged in Go 1.8?

@vstefanovic
Copy link
Member

@pqyptixa
Well, no, the code for 1.8 has been feature-freezed quite a while ago.

@pqyptixa
Copy link

pqyptixa commented Jan 11, 2017

@vstefanovic
ah, right... are these patches published somewhere? if they aren't, do you plan to?
thanks a lot for your work.

@vstefanovic
Copy link
Member

@pqyptixa
No, not yet, they will be after 1.8 is released.

@pqyptixa
Copy link

@vstefanovic any news regarding the mips/FP-emulation patches?

@vstefanovic
Copy link
Member

@pqyptixa we've switched to #18162 for the soft-float news; I still expect to submit the code some time next week.

@golang golang locked and limited conversation to collaborators Feb 21, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

10 participants