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: internal compiler error: init .autotmp_0 recorded as live on entry #25966

Closed
ALTree opened this issue Jun 19, 2018 · 9 comments

Comments

Projects
None yet
5 participants
@ALTree
Copy link
Member

commented Jun 19, 2018

$ gotip version
go version devel +c99300229d Mon Jun 18 23:20:57 2018 +0000 linux/amd64

The following program (reduced from a gosmith-generated one):

package p

var F = []func(){
	func() func() { return (func())(nil) }(),
}

var A = []int{}

type ss struct {
	string
	float64
	i int
}

var V = A[ss{}.i]

crashes the tip compiler when building for GOARCH=wasm with optimization disabled:

$ GOOS=js GOARCH=wasm gotip build -gcflags=-N crash.go 
# command-line-arguments
<autogenerated>:1: internal compiler error: internal error: init .autotmp_0 (type ss) recorded as live on entry

goroutine 34 [running]:
runtime/debug.Stack(0x0, 0x0, 0x0)
	/home/alberto/go/src/runtime/debug/stack.go:24 +0xa7
cmd/compile/internal/gc.Fatalf(0xcc3c19, 0x2f, 0xc000535968, 0x2, 0x2)
	/home/alberto/go/src/cmd/compile/internal/gc/subr.go:182 +0x1f7
cmd/compile/internal/gc.(*Liveness).epilogue(0xc000502420)
	/home/alberto/go/src/cmd/compile/internal/gc/plive.go:1159 +0x1642
cmd/compile/internal/gc.liveness(0xc000500000, 0xc000502000, 0x0, 0x0, 0x0)
	/home/alberto/go/src/cmd/compile/internal/gc/plive.go:1690 +0xc4
cmd/compile/internal/gc.genssa(0xc000502000, 0xc0005181c0)
	/home/alberto/go/src/cmd/compile/internal/gc/ssa.go:4777 +0xbe
cmd/compile/internal/gc.compileSSA(0xc0004f6000, 0x0)
	/home/alberto/go/src/cmd/compile/internal/gc/pgen.go:269 +0x1bc
cmd/compile/internal/gc.compileFunctions.func2(0xc0004d8a80, 0xc0004b01b0, 0x0)
	/home/alberto/go/src/cmd/compile/internal/gc/pgen.go:323 +0x49
created by cmd/compile/internal/gc.compileFunctions
	/home/alberto/go/src/cmd/compile/internal/gc/pgen.go:321 +0x11a

@ALTree ALTree added this to the Go1.11 milestone Jun 19, 2018

@ALTree ALTree changed the title cmd/compile: internal compiler error: init .autotmp_0 (type ss) recorded as live on entry cmd/compile: internal compiler error: init .autotmp_0 recorded as live on entry Jun 19, 2018

@ALTree

This comment has been minimized.

Copy link
Member Author

commented Jun 19, 2018

@neelance

This comment has been minimized.

Copy link
Member

commented Jun 20, 2018

I've investigated this a bit, but I'm not sure on the details of the liveness analysis.

@aclements Question: How does VarDef relate to other operations, especially Addr? Is the following a valid order?

v27 (15) = Addr <*ss> {.autotmp_0} v2
v28 (15) = VarDef <mem> {.autotmp_0} v22
@josharian

This comment has been minimized.

Copy link
Contributor

commented Jun 20, 2018

"recorded as live on entry" means that the liveness analysis thinks that a pointer contains a meaningful value (is live) when the function begins, before even considering arguments (on entry). In practice, usually this means that you have a "use before assignment/initialization" problem.

For most of SSA world, Value order is meaningless. VarDef relates to other values through the memory chain (an ordering implicit by values that have a memory argument and values that have memory type). The schedule pass is responsible for putting values in a good order; it should use the memory chain as part of that.

You might want to try running with the SSA check enabled. It can often spot problems in SSA form earlier and help pinpoint them.

@cherrymui

This comment has been minimized.

Copy link
Contributor

commented Jun 20, 2018

It looks to me that we take the address of .autotmp_0 before the VarDef. The liveness code probably considers the autotmp should be live at this point, then back it up to the function entry. The address is used to compute an offset, and the offset is not used until after the VarDef.

v56 (15) = LoweredAddr <*ss> {.autotmp_0} v2
v52 (15) = I64AddConst <*float64> [16] v56 : R2
... // no use of v52 until later

Later,

v28 (15) = VarDef <mem> {.autotmp_0} v22
...
v51 (15) = F64Store <mem> [0] v52 v62 v53 // use of v52

On other architectures, address operation takes offset, and it is rematerializeable, so it is moved down right before use. However, on Wasm, while LoweredAddr is rematerializeable, it doesn't take offset, and the offset calculation (I64AddConst) is not rematerializeable, so it is not moved down. Maybe we should make LoweredAddr take offset.

@neelance

This comment has been minimized.

Copy link
Member

commented Jun 20, 2018

I am still thinking about which compiler phase is responsible. As far as I understand it is not really wrong to have a LoweredAddr and I64AddConst above the VarDef (even after the schedule phase), since both operations don't access the memory. And without mem parameters, we can't really make it end up in a better order. So my current conclusion (without knowing that much) would be that the liveness analysis should be okay with this output and not throw an error.

@cherrymui

This comment has been minimized.

Copy link
Contributor

commented Jun 20, 2018

Yes, I think the generated code is not wrong. It might be possible to change liveness code to not think it live when the address is taken, but until the address is used. I'm not sure. The tricky part is what uses are considered make it live? Load/Store should be. Offset calculation is not, but it needs to propagate the value. Passing address to a Call should make it live. What about Phi? Anything else?

@neelance

This comment has been minimized.

Copy link
Member

commented Jun 21, 2018

@cherrymui Are you looking into this further / working on a solution? You already know much more about it than I do and there are a few other wasm issues that I'd like to take care of.

@cherrymui

This comment has been minimized.

Copy link
Contributor

commented Jun 21, 2018

I'm not working on this right now. I could take a look, maybe tomorrow.

I think for Go 1.11 we probably want to have some simple workaround, if possible. If we want to change the liveness code, it's gonna be complicated. As we're late in the freeze, I would suggest not to change the liveness code now, and we can revisit in Go 1.12.

@gopherbot

This comment has been minimized.

Copy link

commented Jun 23, 2018

Change https://golang.org/cl/120599 mentions this issue: cmd/compile: fold offset into address on Wasm

@gopherbot gopherbot closed this in 1d303a0 Jun 27, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.