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: regression in writebarrier pass #19179

Closed
dsnet opened this issue Feb 18, 2017 · 8 comments

Comments

Projects
None yet
4 participants
@dsnet
Copy link
Member

commented Feb 18, 2017

c4ef597 caused a regression where the following coder no longer compiles:

type Foo struct{ A, B time.Duration }

func Bar(fs []Foo) string {
	ss := make([]string, 2*len(fs))
	for i, h := range fs {
		ss[2*i] = fmt.Sprintf("%v %v", h.A, h.B.Seconds())
		ss[2*i+1] = fmt.Sprintf("%v %v", h.A, h.B.Seconds())
	}
	return strings.Join(ss, ",")
}

It now panics with:

./main.go:16: internal compiler error: attempt to load unspilled value v53 = MOVQload <time.Duration> {h} [8] v2 v117

goroutine 1 [running]:
runtime/debug.Stack(0x0, 0x0, 0x0)
	/home/rawr/Projects/go/src/runtime/debug/stack.go:24 +0x79
cmd/compile/internal/gc.Fatalf(0xaccaeb, 0x22, 0xc4204a4600, 0x1, 0x1)
	/home/rawr/Projects/go/src/cmd/compile/internal/gc/subr.go:175 +0x230
cmd/compile/internal/gc.(*ssaExport).Fatalf(0xdd0af5, 0x100d00000001, 0xaccaeb, 0x22, 0xc4204a4600, 0x1, 0x1)
	/home/rawr/Projects/go/src/cmd/compile/internal/gc/ssa.go:4929 +0x67
cmd/compile/internal/ssa.(*Config).Fatalf(0xc420452000, 0x100d00000001, 0xaccaeb, 0x22, 0xc4204a4600, 0x1, 0x1)
	/home/rawr/Projects/go/src/cmd/compile/internal/ssa/config.go:345 +0x76
cmd/compile/internal/ssa.(*Func).Fatalf(0xc4204b4000, 0xaccaeb, 0x22, 0xc4204a4600, 0x1, 0x1)
	/home/rawr/Projects/go/src/cmd/compile/internal/ssa/func.go:416 +0x72
cmd/compile/internal/ssa.(*regAllocState).allocValToReg(0xc4204d83c0, 0xc420453988, 0xffce, 0x100000001, 0xc400001233, 0xc4204551e8)
	/home/rawr/Projects/go/src/cmd/compile/internal/ssa/regalloc.go:454 +0x58e
cmd/compile/internal/ssa.(*regAllocState).regalloc(0xc4204d83c0, 0xc4204b4000)
	/home/rawr/Projects/go/src/cmd/compile/internal/ssa/regalloc.go:1188 +0x1731
cmd/compile/internal/ssa.regalloc(0xc4204b4000)
	/home/rawr/Projects/go/src/cmd/compile/internal/ssa/regalloc.go:135 +0x62
cmd/compile/internal/ssa.Compile(0xc4204b4000)
	/home/rawr/Projects/go/src/cmd/compile/internal/ssa/compile.go:70 +0x2c4
cmd/compile/internal/gc.buildssa(0xc4200b9180, 0x0)
	/home/rawr/Projects/go/src/cmd/compile/internal/gc/ssa.go:173 +0x1060
cmd/compile/internal/gc.compile(0xc4200b9180)
	/home/rawr/Projects/go/src/cmd/compile/internal/gc/pgen.go:366 +0x2d0
cmd/compile/internal/gc.funccompile(0xc4200b9180)
	/home/rawr/Projects/go/src/cmd/compile/internal/gc/dcl.go:1226 +0xdc
cmd/compile/internal/gc.Main()
	/home/rawr/Projects/go/src/cmd/compile/internal/gc/main.go:473 +0x202d
main.main()
	/home/rawr/Projects/go/src/cmd/compile/main.go:50 +0x101

\cc @cherrymui @dr2chase @josharian

@dsnet dsnet added the NeedsFix label Feb 18, 2017

@dsnet dsnet added this to the Go1.9 milestone Feb 18, 2017

@cherrymui cherrymui self-assigned this Feb 19, 2017

@gopherbot

This comment has been minimized.

Copy link

commented Feb 19, 2017

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

@funny-falcon

This comment has been minimized.

Copy link
Contributor

commented Feb 21, 2017

Catched with same bit in code with defer.
Code looks like:

    func (bld *DbBuilder) Finish() error {
        defer bld.Unlink() // <- here is an error
        // ... other code
    }

Unfortunately, can't build small reproduce case.

@funny-falcon

This comment has been minimized.

Copy link
Contributor

commented Feb 21, 2017

Changeset 37250 doesn't fix for me at the moment.

go version devel +5bbe1ce Sat Feb 18 19:44:10 2017 -0500 linux/amd64
$ go build
./builder.go:239: internal compiler error: attempt to load unspilled value v1780 = Phi <int> v282 v449
....
@funny-falcon

This comment has been minimized.

Copy link
Contributor

commented Feb 21, 2017

I reduced case to minimal. Looks like removing any line makes it compile

package dbfile

import (
	"encoding/binary"
)

type DbBuilder struct {
	arr        []int
}

func (bld *DbBuilder) Finish() error {
	defer bld.Finish()

	var hash []byte
	for _, ixw := range bld.arr {
		for {
			if ixw != 0 {
				return nil
			}
			ixw--
		insertOne:
			for {
				for i := 0; i < 1; i++ {
					if binary.LittleEndian.Uint16(hash[i:]) == 0 {
						break insertOne
					}
				}
			}
		}
	}

	return nil
}
@cherrymui

This comment has been minimized.

Copy link
Contributor

commented Feb 21, 2017

@funny-falcon Thanks for the repro! However, the related values are not touched in the writebarrier pass, and they are in the correct order at that point. Later the tighten pass got something wrong. So it's a different bug. I'll take a look.

@funny-falcon

This comment has been minimized.

Copy link
Contributor

commented Feb 21, 2017

@cherrymui panic("") could be changed to any non-empty line: ixw = 0, return nil or break - all still trigger error. But removing that line makes it compileable.

@cherrymui

This comment has been minimized.

Copy link
Contributor

commented Feb 21, 2017

@funny-falcon, I filed issue #19217, since it is a different bug, and even apply to Go 1.8.

@funny-falcon

This comment has been minimized.

Copy link
Contributor

commented Feb 21, 2017

@cherrymui, thank you

@gopherbot gopherbot closed this in a355639 Feb 21, 2017

@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.
You can’t perform that action at this time.