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

x/build/cmd/release: linux-s390x-crosscompile builder fails to compile Go 1.16 #43090

Closed
dmitshur opened this issue Dec 9, 2020 · 12 comments
Closed
Labels
arch-s390x Issues solely affecting the s390x architecture. Builders x/build issues (builders, bots, dashboards) FrozenDueToAge NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. release-blocker
Milestone

Comments

@dmitshur
Copy link
Contributor

dmitshur commented Dec 9, 2020

The linux-s390x-crosscompile builder is used in the release process to build s390x.tar.gz release archives. It currently fails when building Go 1.16 (with go1.4.3 linux/amd64 as the bootstrap version).

This can be reproduced if your account has permissions needed to run releasebot (documented here) with:

# (The release command uses builders to create a release artifact locally.
#  It does not publish anything, so it's safe to run for testing needs.)
$ release -target=linux-s390x -version=go1.16alpha0 -rev=master  # master was 854a2f8e01a554d8052445563863775406a04b71
2020/12/08 23:33:29 linux-s390x: Start.
2020/12/08 23:33:29 linux-s390x: Creating buildlet.
2020/12/08 23:34:01 linux-s390x: Pushing source to buildlet.
2020/12/08 23:34:08 linux-s390x: Writing VERSION file.
2020/12/08 23:34:08 linux-s390x: Cleaning goroot (pre-build).
2020/12/08 23:34:09 linux-s390x: Building (make.bash only).
2020/12/08 23:34:15 linux-s390x: Error: Build failed: exit status 2
Output:
Building Go cmd/dist using /go1.4. (go1.4.3 linux/amd64)
Building Go toolchain1 using /go1.4.
# bootstrap/cmd/compile/internal/ssa
/workdir/go/src/cmd/compile/internal/ssa/rewriteS390X.go:12979[/workdir/go/pkg/bootstrap/src/bootstrap/cmd/compile/internal/ssa/rewriteS390X.go:12983]: invalid operation: 18446744073709551615 << c (shift count type int8, must be unsigned integer)
/workdir/go/src/cmd/compile/internal/ssa/rewriteS390X.go:12983[/workdir/go/pkg/bootstrap/src/bootstrap/cmd/compile/internal/ssa/rewriteS390X.go:12987]: invalid operation: 18446744073709551615 << c (shift count type int8, must be unsigned integer)
/workdir/go/src/cmd/compile/internal/ssa/rewriteS390X.go:12997[/workdir/go/pkg/bootstrap/src/bootstrap/cmd/compile/internal/ssa/rewriteS390X.go:13001]: invalid operation: 18446744073709551615 >> c (shift count type int8, must be unsigned integer)
/workdir/go/src/cmd/compile/internal/ssa/rewriteS390X.go:13001[/workdir/go/pkg/bootstrap/src/bootstrap/cmd/compile/internal/ssa/rewriteS390X.go:13005]: invalid operation: 18446744073709551615 >> c (shift count type int8, must be unsigned integer)
go tool dist: FAILED: /go1.4/bin/go install -gcflags=-l -tags=math_big_pure_go compiler_bootstrap bootstrap/cmd/...: exit status 2

In contrast, it works okay on release-branch.go1.15:


$ release -target=linux-s390x -version=go1.15alpha0 -watch -rev=release-branch.go1.15
2020/12/08 23:34:20 linux-s390x: Start.
2020/12/08 23:34:20 linux-s390x: Creating buildlet.
2020/12/08 23:34:56 linux-s390x: Pushing source to buildlet.
2020/12/08 23:35:03 linux-s390x: Writing VERSION file.
2020/12/08 23:35:03 linux-s390x: Cleaning goroot (pre-build).
2020/12/08 23:35:04 linux-s390x: Building (make.bash only).
2020/12/09 04:35:04 unsupported GOARCH s390x
Building Go cmd/dist using /go1.4. ()
Building Go toolchain1 using /go1.4.
Building Go bootstrap cmd/go (go_bootstrap) using Go toolchain1.
Building Go toolchain2 using go_bootstrap and Go toolchain1.
Building Go toolchain3 using go_bootstrap and Go toolchain2.
Building packages and commands for host, linux/amd64.
Building packages and commands for target, linux/s390x.
---
Installed Go for linux/s390x in /workdir/go
Installed commands in /workdir/go/bin

The binaries expect /workdir/go to be copied or moved to /usr/local/go
2020/12/08 23:37:52 linux-s390x: Cleaning goroot (post-build).
2020/12/08 23:37:53 linux-s390x: Pushing and running releaselet.
2020/12/08 23:37:53 linux-s390x: Cleaning workdir.
2020/12/08 23:37:54 linux-s390x: Downloading tarball.
2020/12/08 23:38:07 linux-s390x: Wrote "/tmp/go-release-staging_141661091/go1.15alpha0.linux-s390x.tar.gz.untested".
2020/12/08 23:38:07 linux-s390x: Skipping all.bash tests.
2020/12/08 23:38:07 linux-s390x: Moving "/tmp/go-release-staging_141661091/go1.15alpha0.linux-s390x.tar.gz.untested" to "go1.15alpha0.linux-s390x.tar.gz".
2020/12/08 23:38:07 linux-s390x: Done.

The problem may be on the side of the builder, or in the tree (or both). Thoughts?

CC @golang/release, @randall77, @ianlancetaylor, @ruixin-bao, @rajaskakodkar.

@dmitshur dmitshur added NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. release-blocker arch-s390x Issues solely affecting the s390x architecture. labels Dec 9, 2020
@dmitshur dmitshur added this to the Go1.16 milestone Dec 9, 2020
@gopherbot gopherbot added the Builders x/build issues (builders, bots, dashboards) label Dec 9, 2020
@dmitshur
Copy link
Contributor Author

dmitshur commented Dec 9, 2020

This seems related to Go 1.4.3 being used as the bootstrap version. I can reproduce on a Linux machine with:

# Start with Go 1.4.3 as bootstrap Go version (as linux-s390x-crosscompile does).

$ export GOROOT_BOOTSTRAP=$(go1.4.3 env GOROOT)

# First see if we can build linux/s390x at Go 1.15.

$ git co release-branch.go1.15
Switched to branch 'release-branch.go1.15'
Your branch is up-to-date with 'origin/release-branch.go1.15'.
$ GOARCH=s390x GOHOSTARCH=amd64 ./make.bash
2020/12/09 15:33:25 unsupported GOARCH s390x
Building Go cmd/dist using /home/dmitshur/sdk/go1.4.3. ()
Building Go toolchain1 using /home/dmitshur/sdk/go1.4.3.
Building Go bootstrap cmd/go (go_bootstrap) using Go toolchain1.
Building Go toolchain2 using go_bootstrap and Go toolchain1.
Building Go toolchain3 using go_bootstrap and Go toolchain2.
Building packages and commands for host, linux/amd64.
Building packages and commands for target, linux/s390x.
---
Installed Go for linux/s390x in /home/dmitshur/gotip
Installed commands in /home/dmitshur/gotip/bin

# It succeeds.

# Check out master branch (Go 1.16 in development) and try again.

$ git co master
Switched to branch 'master'
Your branch is up-to-date with 'origin/master'.
$ GOARCH=s390x GOHOSTARCH=amd64 ./make.bash
Building Go cmd/dist using /home/dmitshur/sdk/go1.4.3. (go1.4.3 linux/amd64)
Building Go toolchain1 using /home/dmitshur/sdk/go1.4.3.
# bootstrap/cmd/compile/internal/ssa
/home/dmitshur/gotip/src/cmd/compile/internal/ssa/rewriteS390X.go:12979[/home/dmitshur/gotip/pkg/bootstrap/src/bootstrap/cmd/compile/internal/ssa/rewriteS390X.go:12983]: invalid operation: 18446744073709551615 << c (shift count type int8, must be unsigned integer)
/home/dmitshur/gotip/src/cmd/compile/internal/ssa/rewriteS390X.go:12983[/home/dmitshur/gotip/pkg/bootstrap/src/bootstrap/cmd/compile/internal/ssa/rewriteS390X.go:12987]: invalid operation: 18446744073709551615 << c (shift count type int8, must be unsigned integer)
/home/dmitshur/gotip/src/cmd/compile/internal/ssa/rewriteS390X.go:12997[/home/dmitshur/gotip/pkg/bootstrap/src/bootstrap/cmd/compile/internal/ssa/rewriteS390X.go:13001]: invalid operation: 18446744073709551615 >> c (shift count type int8, must be unsigned integer)
/home/dmitshur/gotip/src/cmd/compile/internal/ssa/rewriteS390X.go:13001[/home/dmitshur/gotip/pkg/bootstrap/src/bootstrap/cmd/compile/internal/ssa/rewriteS390X.go:13005]: invalid operation: 18446744073709551615 >> c (shift count type int8, must be unsigned integer)
go tool dist: FAILED: /home/dmitshur/sdk/go1.4.3/bin/go install -gcflags=-l -tags=math_big_pure_go compiler_bootstrap bootstrap/cmd/...: exit status 2

# It fails.

# Now try with Go 1.15.6 as bootstrap.

$ export GOROOT_BOOTSTRAP=$(go1.15.6 env GOROOT)
$ GOARCH=s390x GOHOSTARCH=amd64 ./make.bash
Building Go cmd/dist using /usr/local/go. (go1.15.6 linux/amd64)
Building Go toolchain1 using /usr/local/go.
Building Go bootstrap cmd/go (go_bootstrap) using Go toolchain1.
Building Go toolchain2 using go_bootstrap and Go toolchain1.
Building Go toolchain3 using go_bootstrap and Go toolchain2.
Building packages and commands for host, linux/amd64.
Building packages and commands for target, linux/s390x.
---
Installed Go for linux/s390x in /home/dmitshur/gotip
Installed commands in /home/dmitshur/gotip/bin

# Works.

So the question is: is it unexpected that Go 1.16 no longer bootstraps with Go 1.4.3 (unlike Go 1.15.x, which does)? Or should the cross-compile builder be updated to use a newer bootstrap version?

@bcmills
Copy link
Member

bcmills commented Dec 9, 2020

The current Go toolchain should build with a Go 1.4 bootstrap. (I think @mdempsky has suggested bumping that minimum version higher, but I don't see a concrete proposal to do so yet.)

@bcmills
Copy link
Member

bcmills commented Dec 9, 2020

From the error message, I believe you will find that this failure bisects to CL 233317, and the fix may be to update the rules at S390X.rules:754 to explicitly convert c to type uint8.

@ruixin-bao
Copy link
Contributor

ruixin-bao commented Dec 9, 2020

Hi @bcmills and @dmitshur, thanks for the information and investigation. Yea, from our side, we also found that CL 233317 is the issue.

fix may be to update the rules at S390X.rules:754 to explicitly convert c to type uint8.

Thanks, I will try this fix.

Another thing I noticed is that if I run make.bash against that CL with go1.13.8, there won't be a compilation failure shown above. (EDIT: sorry, I missed Dmitri's comment with successful compilation using go1.15.6)

#:~/work/go/src$ which go
/usr/bin/go
#:~/work/go/src$ go version
go version go1.10.4 linux/s390x
peterbao@shipload1:~/work/go/src$ ./make.bash 
Building Go cmd/dist using /usr/lib/go-1.10. (go1.10.4 linux/s390x)
Building Go toolchain1 using /usr/lib/go-1.10.
# bootstrap/cmd/compile/internal/ssa
/home/peterbao/work/go/src/cmd/compile/internal/ssa/rewriteS390X.go:12979: invalid operation: ^uint64(0) << c (shift count type int8, must be unsigned integer)
/home/peterbao/work/go/src/cmd/compile/internal/ssa/rewriteS390X.go:12983: invalid operation: ^uint64(0) << c (shift count type int8, must be unsigned integer)
/home/peterbao/work/go/src/cmd/compile/internal/ssa/rewriteS390X.go:12997: invalid operation: ^uint64(0) >> c (shift count type int8, must be unsigned integer)
/home/peterbao/work/go/src/cmd/compile/internal/ssa/rewriteS390X.go:13001: invalid operation: ^uint64(0) >> c (shift count type int8, must be unsigned integer)
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
#:~/work/go/src$ export PATH=/usr/local/go/bin:$PATH
#: ~/work/go/src$ go version
go version go1.13.8 linux/s390x
#:~/work/go/src$ ./make.bash 
Building Go cmd/dist using /usr/local/go. (go1.13.8 linux/s390x)
Building Go toolchain1 using /usr/local/go.
Building Go bootstrap cmd/go (go_bootstrap) using Go toolchain1.
Building Go toolchain2 using go_bootstrap and Go toolchain1.
Building Go toolchain3 using go_bootstrap and Go toolchain2.
Building packages and commands for linux/s390x. 
---
Installed Go for linux/s390x in /home/peterbao/work/go
Installed commands in /home/peterbao/work/go/bin

@randall77
Copy link
Contributor

Another option is to change the aux type of SLDconst and friends to uint8 instead of int8.

@ruixin-bao
Copy link
Contributor

ruixin-bao commented Dec 9, 2020

Hi,

the fix may be to update the rules at S390X.rules:754 to explicitly convert c to type uint8.

This does look to fix the compilation issue. I am now able to build master with go1.10.4 with updated rules applied on s390x. We are trying to cross-compile with go1.4.3 and will report back once we have the results.

diff --git a/src/cmd/compile/internal/ssa/gen/S390X.rules b/src/cmd/compile/internal/ssa/gen/S390X.rules
index 39949edbc2..85128b0ef1 100644
--- a/src/cmd/compile/internal/ssa/gen/S390X.rules
+++ b/src/cmd/compile/internal/ssa/gen/S390X.rules
@@ -751,8 +751,8 @@
 // is valid to merge the shift mask (^(uint64(0)<<c)) into the selected
 // bits mask (i.e. that the resultant mask is non-zero and contiguous).
 //
-(RISBGZ (SLDconst x [c]) {r}) && r.InMerge(^uint64(0)<<c) != nil => (RISBGZ x {(*r.InMerge(^uint64(0)<<c)).RotateLeft(c)})
-(RISBGZ (SRDconst x [c]) {r}) && r.InMerge(^uint64(0)>>c) != nil => (RISBGZ x {(*r.InMerge(^uint64(0)>>c)).RotateLeft(-c)})
+(RISBGZ (SLDconst x [c]) {r}) && r.InMerge(^uint64(0)<<uint8(c)) != nil => (RISBGZ x {(*r.InMerge(^uint64(0)<<uint8(c))).RotateLeft(c)})
+(RISBGZ (SRDconst x [c]) {r}) && r.InMerge(^uint64(0)>>uint8(c)) != nil => (RISBGZ x {(*r.InMerge(^uint64(0)>>uint8(c))).RotateLeft(-c)})

Another option is to change the aux type of SLDconst and friends to uint8 instead of int8.

Thanks, I had a try with this approach, it looks like uint8 is not a supported auxint? Adding UInt8 to the function opHasAuxInt seems to create a lot of auxIntToInvalid functions in rewriteS390X.go

@randall77
Copy link
Contributor

Thanks, I had a try with this approach, it looks like uint8 is not a supported auxint? Adding UInt8 to the function opHasAuxInt seems to create a lot of auxIntToInvalid functions in rewriteS390X.go

That may be. Adding it shouldn't be hard (but may be a bit tedious).

@ruixin-bao
Copy link
Contributor

Hi,

the fix may be to update the rules at S390X.rules:754 to explicitly convert c to type uint8.

Both @rajaskakodkar and I have tested that this fixes the cross-compilation problem listed in this issue.

:~/work/go/src$ go version
go version go1.4.3 linux/amd64
:~/work/go/src$ uname -i
x86_64
:~/work/go/src$ git log --pretty=oneline -n 2
c9999a0667a93b6ed628354223240cfc0dc52077 (HEAD -> master) cmd/compile: issue43090 placeholder commit <--- this is the fix commit
854a2f8e01a554d8052445563863775406a04b71 (origin/master, origin/HEAD, issue43090) net/http: add connections back that haven't been canceled
:~/work/go/src$ GOARCH=s390x GOHOSTARCH=amd64 ./make.bash
Building Go cmd/dist using /usr/local/go. (go1.4.3 linux/amd64)
Building Go toolchain1 using /usr/local/go.
Building Go bootstrap cmd/go (go_bootstrap) using Go toolchain1.
Building Go toolchain2 using go_bootstrap and Go toolchain1.
Building Go toolchain3 using go_bootstrap and Go toolchain2.
Building packages and commands for host, linux/amd64.
Building packages and commands for target, linux/s390x.
---
Installed Go for linux/s390x in /home/peterbao/work/go
Installed commands in /home/peterbao/work/go/bin

@ruixin-bao
Copy link
Contributor

Another option is to change the aux type of SLDconst and friends to uint8 instead of int8.
That may be. Adding it shouldn't be hard (but may be a bit tedious).

Thanks, I have explored this approach a bit more. I seem to get it working, but it does need a bit more changes. I am planning to send two CLs soon with both approaches and see which one could fit better.

@dmitshur
Copy link
Contributor Author

Thank you @ruixin-bao and @rajaskakodkar. Please include a "Fixes #43090" (or "For #43090") line in the CLs when you send them, so that this issue is updated when they become available for review.

@gopherbot
Copy link

Change https://golang.org/cl/277078 mentions this issue: cmd/compile: fix incorrect shift count type with s390x rules

@gopherbot
Copy link

Change https://golang.org/cl/277079 mentions this issue: cmd/compile: fix incorrect shift count type with s390x rules

@golang golang locked and limited conversation to collaborators Dec 14, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
arch-s390x Issues solely affecting the s390x architecture. Builders x/build issues (builders, bots, dashboards) FrozenDueToAge NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. release-blocker
Projects
None yet
Development

No branches or pull requests

5 participants