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: unnecessary bounds check in a loop passed as a closure #39756

Open
jordanlewis opened this issue Jun 22, 2020 · 1 comment
Open

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

jordanlewis opened this issue Jun 22, 2020 · 1 comment
Labels
compiler/runtime Issues related to the Go compiler and/or runtime. NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. Performance
Milestone
Unplanned

Comments

@jordanlewis
Copy link

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
Copy link
Contributor

@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.

@randall77 randall77 added this to the Unplanned milestone Jun 22, 2020
@randall77 randall77 added the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Jun 22, 2020
@gopherbot gopherbot added the compiler/runtime Issues related to the Go compiler and/or runtime. label Jul 13, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
compiler/runtime Issues related to the Go compiler and/or runtime. NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. Performance
Projects
None yet
Milestone
Unplanned
Development

No branches or pull requests
3 participants
@jordanlewis @randall77 @gopherbot