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

[dmitshur]

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]

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]

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]

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]

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]

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

[ruixin-bao]

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]

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]

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]

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]

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]

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

[gopherbot]

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