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

Jump to bottom

cmd/compile: internal compiler error: width not calculated: map[string]string #44895

Closed
andig opened this issue Mar 9, 2021 · 7 comments
Closed

cmd/compile: internal compiler error: width not calculated: map[string]string #44895

andig opened this issue Mar 9, 2021 · 7 comments
Labels
FrozenDueToAge NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Milestone
Go1.17

Comments

@andig
Copy link
Contributor

andig commented Mar 9, 2021 •
edited

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

$ go version
go version devel +48ddf70 Tue Mar 9 20:35:41 2021 +0000 darwin/arm64

Does this issue reproduce with the latest release?

yes

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

go env Output
$ go env
❯ gotip env
GO111MODULE=""
GOARCH="arm64"
GOBIN=""
GOCACHE="/Users/andig/Library/Caches/go-build"
GOENV="/Users/andig/Library/Application Support/go/env"
GOEXE=""
GOFLAGS=""
GOHOSTARCH="arm64"
GOHOSTOS="darwin"
GOINSECURE=""
GOMODCACHE="/Users/andig/go/pkg/mod"
GONOPROXY=""
GONOSUMDB=""
GOOS="darwin"
GOPATH="/Users/andig/go"
GOPRIVATE=""
GOPROXY="https://proxy.golang.org,direct"
GOROOT="/Users/andig/sdk/gotip"
GOSUMDB="sum.golang.org"
GOTMPDIR=""
GOTOOLDIR="/Users/andig/sdk/gotip/pkg/tool/darwin_arm64"
GOVCS=""
GOVERSION="devel +48ddf70 Tue Mar 9 20:35:41 2021 +0000"
GCCGO="gccgo"
AR="ar"
CC="clang"
CXX="clang++"
CGO_ENABLED="1"
GOMOD="/Users/andig/htdocs/evcc/go.mod"
CGO_CFLAGS="-g -O2"
CGO_CPPFLAGS=""
CGO_CXXFLAGS="-g -O2"
CGO_FFLAGS="-g -O2"
CGO_LDFLAGS="-g -O2"
PKG_CONFIG="pkg-config"
GOGCCFLAGS="-fPIC -arch arm64 -pthread -fno-caret-diagnostics -Qunused-arguments -fmessage-length=0 -fdebug-prefix-map=/var/folders/sv/rs_453y57xj86xsbz3kw1mbc0000gn/T/go-build3964674039=/tmp/go-build -gno-record-gcc-switches -fno-common"

In #43931 I've learned that I can use gotip to experiment with generic code and that (#43931 (comment))

Non-generic Go code is expected to fully work, so please file issues if you find anything that works without -G=3 but breaks with -G=3

I'm using

gotip run -gcflags=all=-G=3 main.go

on this piece of code:

func (v *Volvo) status() (interface{}, error) {
	var res volvoStatus

	req, err := v.request(fmt.Sprintf("%s/vehicles/%s/status", volvoAPI, v.vin))
	if err == nil {
		err = v.DoJSON(req, &res)
	}

	return res, err
}

What did you expect to see?

No error

What did you see instead?

vehicle/volvo.go:172:17: internal compiler error: width not calculated: map[string]string

The entire code is in https://github.com/andig/evcc/tree/go2/registry
@ianlancetaylor ianlancetaylor changed the title gotip: internal compiler error: width not calculated: map[string]string cmd/compile: internal compiler error: width not calculated: map[string]string Mar 10, 2021
@ianlancetaylor
Copy link
Contributor

CC @mdempsky @randall77

@ianlancetaylor
Copy link
Contributor

CC @griesemer

@ianlancetaylor ianlancetaylor added the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Mar 10, 2021
@ianlancetaylor ianlancetaylor added this to the Go1.17 milestone Mar 10, 2021
@danscales
Copy link
Contributor

Yes, thanks for the simple example! I guess in rare cases we still need to do CheckSize in g.typ(), even though it shouldn't really be necessary - I think I should be able to figure out when exactly when/why this is needed.

@mdempsky
Copy link
Member

There's no real harm to extra calls to CheckSize, other than marginally extra CPU usage if we don't end up needing the type (e.g., dead code elimination or something). So feel free to liberally add them as needed to fix issues, as long as we add regress tests for them too.

@cuonglm
Copy link
Member

cuonglm commented Mar 10, 2021

Yes, thanks for the simple example! I guess in rare cases we still need to do CheckSize in g.typ(), even though it shouldn't really be necessary - I think I should be able to figure out when exactly when/why this is needed.

I'm leaning to the same fix as @mdempsky suggested, like what I did in https://go-review.googlesource.com/c/go/+/299689. It's safer to just include the check, because now we don't compile function immediately but pushing to queue and compile later instead. So there's no way to guarantee that the type size is calculated. Previously, it's ok to compute size during SSA generation, now it doesn't.

@mdempsky
Copy link
Member

Previously, it's ok to compute size during SSA generation, now it doesn't.

While we didn't explicitly detect size calculations during non-concurrent SSA construction before, we also don't have any code that made us more aggressive about preemptively checking type sizes when concurrency was enabled. So any late size calculations that used to work, would have failed with concurrent compilations. We just don't have as much test coverage for concurrent compilation, and always using the compilation queue made those mistakes more visible.

If we wanted to restore the old, lax behavior, then I think in gc/compile.go:compileFunctions we could leave types.CalcSizeDisabled and base.Ctxt.InParallel as false when base.Flag.LowerC is 1. In that case, while there are multiple goroutines compiling everything, all of the work is serialized on a 1-element semaphore.

However, I'd lean towards keeping the failures noisy to help rooting them out and fixing them, rather than leaving them as latent issues for users that use concurrent compilation.

andig added a commit to evcc-io/evcc that referenced this issue Mar 10, 2021
@andig
Add generic registry (hides golang/go#44895)
f53c09b
@gopherbot
Copy link

Change https://golang.org/cl/300989 mentions this issue: cmd/compile: call types.CheckSize() in g.typ()

andig added a commit to evcc-io/evcc that referenced this issue Mar 19, 2021
@andig
Add generic registry (hides golang/go#44895)
1143639
@golang golang locked and limited conversation to collaborators Mar 12, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
FrozenDueToAge NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Projects
None yet
Milestone
Go1.17
Development

No branches or pull requests
6 participants
@mdempsky @andig @danscales @ianlancetaylor @cuonglm @gopherbot