cmd/compile: unnecessary bounds check in a loop passed as a closure

[jordanlewis]

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

$ go version

go version go1.13.10 darwin/amd64

Does this issue reproduce with the latest release?

Yes, using amd64 gc (tip) on go.godbolt.org.

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

go env Output

$ go env

GO111MODULE=""
GOARCH="amd64"
GOBIN=""
GOCACHE="/Users/jordan/Library/Caches/go-build"
GOENV="/Users/jordan/Library/Application Support/go/env"
GOEXE=""
GOFLAGS=""
GOHOSTARCH="amd64"
GOHOSTOS="darwin"
GONOPROXY=""
GONOSUMDB=""
GOOS="darwin"
GOPATH="/Users/jordan/go"
GOPRIVATE=""
GOPROXY="https://proxy.golang.org,direct"
GOROOT="/usr/local/Cellar/go@1.13/1.13.10_1/libexec"
GOSUMDB="sum.golang.org"
GOTMPDIR=""
GOTOOLDIR="/usr/local/Cellar/go@1.13/1.13.10_1/libexec/pkg/tool/darwin_amd64"
GCCGO="gccgo"
AR="ar"
CC="clang"
CXX="clang++"
CGO_ENABLED="1"
GOMOD=""
CGO_CFLAGS="-g -O2"
CGO_CPPFLAGS=""
CGO_CXXFLAGS="-g -O2"
CGO_FFLAGS="-g -O2"
CGO_LDFLAGS="-g -O2"
PKG_CONFIG="pkg-config"
GOGCCFLAGS="-fPIC -m64 -pthread -fno-caret-diagnostics -Qunused-arguments -fmessage-length=0 -fdebug-prefix-map=/var/folders/kz/_525lwtd4f7cxsb_qzwp0mv00000gn/T/go-build573559598=/tmp/go-build -gno-record-gcc-switches -fno-common"

What did you do?

I loaded the following program into go.godbolt.org:

package foo

func run(f func ()) {
   f()
}

func foo(c []int64, n int, v int64)  {
   run(func() {
       _ = c[n-1]
       for i := 0; i < n; i++ {
           c[i] = v
       }
   })
}

https://go.godbolt.org/z/iPaz7f

What did you expect to see?

I expected that the bounds check for c[i] would be eliminated because of the early bounds check triggered by c[n-1], the way that it would if the inner loop wasn't sent as a closure to run.

This should be possible because c doesn't get assigned to within the closure.

What did you see instead?

The compiler emits a bounds check for c[i].

[randall77]

@brtzsnr @rasky @zdjones

It's not just the bounds check, it is also reloading the backing store pointer every time. c is somehow being treated as having its address taken. I don't understand why that is.

You can fix this by doing

   run(func() {
       a := c
       _ = a[n-1]
       for i := 0; i < n; i++ {
           a[i] = v
       }
   })

a := c[:n] also works.