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: arm64 build constraint not supported in bootstrapping ? #36551

Closed
shawn-xdji opened this issue Jan 14, 2020 · 8 comments
Closed

cmd/compile: arm64 build constraint not supported in bootstrapping ? #36551

shawn-xdji opened this issue Jan 14, 2020 · 8 comments

Comments

@shawn-xdji
Copy link
Contributor

@shawn-xdji shawn-xdji commented Jan 14, 2020

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

$ go version
branch tip

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


arm64 platform

What did you do?

Add an arch-specific file, say nat_arm64.go, but it's not built during bootstrapping, leading to unexpected build error.

Command line of building the math/big packge:

/usr/lib/go-1.10/pkg/tool/linux_arm64/compile -o $WORK/b089/pkg.a -trimpath $WORK/b089 -p bootstrap/math/big -complete -buildid A-inRWtP3l76bAqzsiqn/A-inRWtP3l76bAqzsiqn -goversion go1.10.4 -D "" -importcfg $WORK/b089/importcfg -pack -c=4 ./accuracy_string.go ./arith.go ./arith_decl_pure.go ./decimal.go ./doc.go ./float.go ./floatconv.go ./floatmarsh.go ./ftoa.go ./int.go ./intconv.go ./intmarsh.go ./nat.go ./natconv.go ./prime.go ./rat.go ./ratconv.go ./ratmarsh.go ./roundingmode_string.go ./sqrt.go

Seems that build constraint for arm64 is not supported by go-1.10?
Tried with amd64 (nat_amd64.go), it works fine.

What did you expect to see?

bootstrapping build support arm64 build constraint, or any workaround solution.

What did you see instead?

build error in "Building Go toolchain1 using /usr/lib/go-1.10." as the arch-specific file is not included.

@shawn-xdji

This comment has been minimized.

Copy link
Contributor Author

@shawn-xdji shawn-xdji commented Jan 14, 2020

As a comparison, command line of building math/big on amd64 where ./nat_amd64.go is included.

/home/xiaji01/util/go/pkg/tool/linux_amd64/compile -o $WORK/b097/pkg.a -trimpath "$WORK/b097=>" -p bootstrap/math/bi g -lang=go1.14 -complete -buildid SuQcvfNw1-wahpUcxV9E/SuQcvfNw1-wahpUcxV9E -D "" -importcfg $WORK/b097/importcfg -pac k -c=4 ./accuracy_string.go ./arith.go ./arith_decl_pure.go ./decimal.go ./doc.go ./float.go ./floatconv.go ./floatmar sh.go ./ftoa.go ./int.go ./intconv.go ./intmarsh.go ./nat.go ./nat_amd64.go ./natconv.go ./prime.go ./rat.go ./ratconv .go ./ratmarsh.go ./roundingmode_string.go ./sqrt.go

Thanks.

@bradfitz

This comment has been minimized.

Copy link
Contributor

@bradfitz bradfitz commented Jan 14, 2020

arm64 has been recognized since Go 1.5.

Please provide a complete repro.

@shawn-xdji

This comment has been minimized.

Copy link
Contributor Author

@shawn-xdji shawn-xdji commented Jan 14, 2020

@bradfitz I got the following error with the enclosed change. Thanks a lot.

Building Go cmd/dist using /usr/lib/go-1.10. (go1.10.4 linux/arm64)
Building Go toolchain1 using /usr/lib/go-1.10.

bootstrap/math/big

/home/xiaji01/src/go.natsqr.arm/src/math/big/nat.go:271: undefined: karatsubaThreshold
/home/xiaji01/src/go.natsqr.arm/src/math/big/nat.go:423: undefined: karatsubaThreshold
/home/xiaji01/src/go.natsqr.arm/src/math/big/nat.go:436: undefined: karatsubaThreshold
/home/xiaji01/src/go.natsqr.arm/src/math/big/nat.go:520: undefined: karatsubaSqrThreshold
/home/xiaji01/src/go.natsqr.arm/src/math/big/nat.go:565: undefined: basicSqrThreshold
/home/xiaji01/src/go.natsqr.arm/src/math/big/nat.go:570: undefined: karatsubaSqrThreshold
/home/xiaji01/src/go.natsqr.arm/src/math/big/nat.go:581: undefined: karatsubaSqrThreshold
go tool dist: FAILED: /usr/lib/go-1.10/bin/go install -gcflags=-l -tags=math_big_pure_go compiler_bootstrap bootstrap/cmd/...: exit status 2

nat.patch.txt

@shawn-xdji

This comment has been minimized.

Copy link
Contributor Author

@shawn-xdji shawn-xdji commented Jan 14, 2020

looks like *_arm64.go is excluded on arm64 platform, if I managed to bypass the error then toolchain1 recognizes them as expected.
(edit: the behavior is not specific to go-1.10)

@bradfitz

This comment has been minimized.

Copy link
Contributor

@bradfitz bradfitz commented Jan 14, 2020

Look at the command that failed; it includes tags=math_big_pure_go.

It's trying to build a non-assembly version first, and your patch deletes the pure Go definitions.

This is working as intended.

@bradfitz bradfitz closed this Jan 14, 2020
@shawn-xdji

This comment has been minimized.

Copy link
Contributor Author

@shawn-xdji shawn-xdji commented Jan 15, 2020

Files with arm64 or wasm suffixes are ignored from bootstrapping build,

// cmd/dist/buildtool.go
112 // File suffixes that use build tags introduced since Go 1.4.
113 // These must not be copied into the bootstrap build directory.
114 var ignoreSuffixes = []string{
115 "_arm64.s",
116 "_arm64.go",
117 "_wasm.s",
118 "_wasm.go",
119 }

159 for _, suf := range ignoreSuffixes {
160 if strings.HasSuffix(name, suf) {
161 continue Dir
162 }
163 }

why are they ignored even they already have "// +build !math_big_pure_g"? Is it for supporting a GOROOT_BOOTSTRAP < Go 1.4? Thanks.

@ianlancetaylor

This comment has been minimized.

Copy link
Contributor

@ianlancetaylor ianlancetaylor commented Jan 15, 2020

The ignoreSuffixes list is because Go 1.4 didn't treat those suffixes specially. If we left them in the bootstrap directory it would break bootstrap with Go 1.4 which would try to compile those files when it shouldn't.

If it's important to make that work we could likely clean this up a bit by only ignoring the files if they have no build tags.

@bradfitz

This comment has been minimized.

Copy link
Contributor

@bradfitz bradfitz commented Jan 15, 2020

@ianlancetaylor, we could also add explicit +build tags to the copies in the bootstrap directory instead of skipping files.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
3 participants
You can’t perform that action at this time.