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: "attempt to load unspilled value" compiling valid code #15084

Closed
josharian opened this issue Apr 3, 2016 · 7 comments
Closed

cmd/compile: "attempt to load unspilled value" compiling valid code #15084

josharian opened this issue Apr 3, 2016 · 7 comments
Milestone

Comments

@josharian
Copy link
Contributor

Attempt to compile:

package x

type T struct {
    i int
    e interface{}
}

func (t *T) F() bool {
    if t.i != 0 {
        return false
    }
    _, ok := t.e.(string)
    return ok
}

var x int

func g(t *T) {
    if t.F() || true {
        if t.F() {
            x = 0
        }
    }
}

Result:

x.go:19: internal compiler error: attempt to load unspilled value v69 = MOVQload <*uint8> [8] v5 v1

git bisect points at 40f2b57, CL 19728.

This is blocking other compiler changes I'm working on.

cc @brtzsnr @dr2chase @randall77

@josharian josharian added this to the Go1.7 milestone Apr 3, 2016
@brtzsnr
Copy link
Contributor

brtzsnr commented Apr 3, 2016

Looks like an issue in the register allocator and that cl is just triggering it. I don't know enough about the register allocator, so I cannot be sure.

@josharian
Copy link
Contributor Author

May be, although it is also possible that this case tickles deadcode into feeding the register allocator invalid SSA. I haven't looked.

In any case, when this is fixed, we should also remove the go:noinline work around for IsSlice (in type.go) introduced in CL 21611.

@gopherbot
Copy link

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

gopherbot pushed a commit that referenced this issue Apr 21, 2016
Ancestor comparison was the wrong way around, effectively
disabling the def-must-dominate-use check.

Update #15084

Change-Id: Ic56d674c5000569d2cc855bbb000a60eae517c7c
Reviewed-on: https://go-review.googlesource.com/22330
Run-TryBot: Keith Randall <khr@golang.org>
Reviewed-by: Josh Bleecher Snyder <josharian@gmail.com>
@gopherbot
Copy link

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

@randall77
Copy link
Contributor

randall77 commented Apr 21, 2016

Combination of:

  1. phielimValue leaving graph with values not dominating uses (https://go-review.googlesource.com/c/22335/)
  2. bug in the dominator graph calculation ( cl/22334 )
  3. bug in check() not detecting 1 or 2 ( cl/22330 )
    All in all, a very nice test case.

@dr2chase
Copy link
Contributor

Should be fixed, see https://go-review.googlesource.com/#/c/22347/

@gopherbot
Copy link

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

@golang golang locked and limited conversation to collaborators Apr 25, 2017
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

5 participants