Skip to content

cmd/compile: teach BCE about min builtin #68553

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

Open
tdakkota opened this issue Jul 23, 2024 · 3 comments
Open

cmd/compile: teach BCE about min builtin #68553

tdakkota opened this issue Jul 23, 2024 · 3 comments
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

Comments

@tdakkota
Copy link
Contributor

tdakkota commented Jul 23, 2024

Go version

go version go1.22.5 linux/amd64

Output of go env in your module/workspace:

GO111MODULE=''
GOARCH='amd64'
GOBIN=''
GOCACHE='/home/tdakkota/.cache/go-build'
GOENV='/home/tdakkota/.config/go/env'
GOEXE=''
GOEXPERIMENT=''
GOFLAGS=''
GOHOSTARCH='amd64'
GOHOSTOS='linux'
GOINSECURE=''
GOMODCACHE='/home/tdakkota/go/pkg/mod'
GONOPROXY=''
GONOSUMDB=''
GOOS='linux'
GOPATH='/home/tdakkota/go'
GOPRIVATE=''
GOPROXY='https://proxy.golang.org,direct'
GOROOT='/home/tdakkota/go/pkg/mod/golang.org/toolchain@v0.0.1-go1.22.5.linux-amd64'
GOSUMDB='sum.golang.org'
GOTMPDIR=''
GOTOOLCHAIN='go1.22.5+auto'
GOTOOLDIR='/home/tdakkota/go/pkg/mod/golang.org/toolchain@v0.0.1-go1.22.5.linux-amd64/pkg/tool/linux_amd64'
GOVCS=''
GOVERSION='go1.22.5'
GCCGO='gccgo'
GOAMD64='v1'
AR='ar'
CC='gcc'
CXX='g++'
CGO_ENABLED='1'
GOMOD='/dev/null'
GOWORK=''
CGO_CFLAGS='-O2 -g'
CGO_CPPFLAGS=''
CGO_CXXFLAGS='-O2 -g'
CGO_FFLAGS='-O2 -g'
CGO_LDFLAGS='-O2 -g'
PKG_CONFIG='pkg-config'
GOGCCFLAGS='-fPIC -m64 -pthread -Wl,--no-gc-sections -fmessage-length=0 -ffile-prefix-map=/tmp/go-build2547774337=/tmp/go-build -gno-record-gcc-switches'

What did you do?

for i := range min(len(keys), len(values)) {
    println(keys[i]) // Found IsInBounds
    println(values[i]) // Found IsInBounds
}

https://go.dev/play/p/wE9DX4D3Iga
https://go.godbolt.org/z/11PYfc6x3

What did you see happen?

Bound checks are not eliminated.

What did you expect to see?

Given that min(len(x), len(y)) is non-negative and less or equal than len(x) and len(y) , i is always in bounds of both slices, so bounds checks could be eliminated.

Iteration patterns like this are quite common, especially in columnar data formats.

@gopherbot gopherbot added the compiler/runtime Issues related to the Go compiler and/or runtime. label Jul 23, 2024
@randall77 randall77 added this to the Unplanned milestone Jul 23, 2024
@dmitshur dmitshur added the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Jul 25, 2024
@tdakkota
Copy link
Contributor Author

It seems like CL 622240 mostly solved this issue:

$ go version
go version devel go1.24-4b30a40d88 Tue Oct 29 16:47:05 2024 +0000 linux/amd64
$ go tool compile -d=ssa/check_bce/debug=1 test.go
test.go:5:21: Found IsInBounds
func iter(keys, values []string) {
    for i := 0; i < min(len(keys), len(values)); i++ {
        println(keys[i]) // Found IsInBounds
        println(values[i])
    }
}

func iter2(keys, values []string) {
    l := min(len(keys), len(values))
    for i := range l {
        println(keys[i])
        println(values[i])
    }
}

@zigo101
Copy link

zigo101 commented Oct 30, 2024

No bound checks in the following code

func iter(keys, values []string) {
    l := min(len(keys), len(values))
    for i := 0; i < l; i++ {
        println(keys[i])
        println(values[i])
    }
}

func iter2(keys, values []string) {
    for i := range min(len(keys), len(values)) {
        println(keys[i])
        println(values[i])
    }
}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to 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
Projects
None yet
Development

No branches or pull requests

6 participants