-
Notifications
You must be signed in to change notification settings - Fork 18k
cmd/compile: bce: if slice[n] is in bounds, then slice[:n+1] should have bounds check eliminated #32431
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
Comments
Need to watch out for blen==MinInt. Then the index succeeds but the slice fails. |
@randall77 While not immediately useful, I think it's worth noting that this would no longer be a concern if we adopted #19623 or #31500. |
I think the titles suggestion is not safe for when n==MaxInt as n+1 would overflow. |
It looks Go 1.12 does BCE for this case. Some finding: package main
type T = uint64
func f(s []int, n T) {
_ = s[n]
_ = s[:n+1] // BCEed when T is int/uint/int64/uint64 for Go SDK 1.12, but not for tip.
// I think T=unsignedIntergerType should be ok here.
}
func g(s []int, n T) {
_ = s[n-1]
_ = s[:n] // BCEed when T is int/uint/int64/uint64 for Go SDK 1.12, but not for tip.
// I think T=unsignedIntergerType should be ok here.
}
func main() {} [edit]: If Go specification can specify (, or gc can limit) that the lengths of arrays/slices/strings may not exceed |
@ugorji It seems the bounds check in your example is removed, at least as of 1.12.6. Do you have an example where the bounds check is not removed? Otherwise this can be closed, fixed somewhere <= 1.12.6 |
@zdjones did you check the behaviour is correct when n==MaxIntT? |
It is impossible for a program to successfully allocate a slice with lengh n == MaxIntT for T is int/uint/int64/uint64, even if the element type is byte. For other signed interger types, if n==MaxIntT, then n+1 is negative, so the bound check can't be removed for sure. For unsigned integer types, n+1 is zero, so the bound check can be removed for sure. In short, bound check can be removed for int, int64 and any unsigned integer types. |
Timed out in state WaitingForInfo. Closing. (I am just a bot, though. Please speak up if this is a mistake or you have the requested information.) |
This still needs more thought, don't close it yet. I've started checking the current behaviour near MaxInt and ended up down quite the rabbit-hole (exploring how big of a slice the compiler and runtime will allow, compared with what the spec allows for) |
Note that you can make slices of 0-sized items. That will let you test near maxint without using huge amounts of memory.
|
With @randall77's comment, the conclusion in my last comment is wrong (for int and int64). |
It looks the slice expression in the following code has already BCEed at least since Go 1.12. _ = b[blen-1] // bounds check: Found IsInBounds (expected)
use(b[:blen]) // BCEed So maybe it also should for OP's code: b[blen-1] = '"' // bounds check: Found IsInBounds (expected)
use(b[:blen]) // bounds check: Found IsSliceInBounds (unexpected as above is sufficient) Rethought it for a while, even if |
And it looks this problem only happens for package-level slices or indexes: package main
var blen int = 1
var b []byte = make([]byte, 1)
var b2 []byte
func fff(b []byte, blen int) {
type _ int
b[blen-1] = '"' // Found IsInBounds
b2 = b[:blen]
}
func ggg() {
type _ int
b[blen-1] = '"' // Found IsInBounds
b2 = b[:blen] // Found IsSliceInBounds
}
func ggg2(b []byte) {
type _ int
b[blen-1] = '"' // Found IsInBounds
b2 = b[:blen] // Found IsSliceInBounds
}
func ggg3(blen int) {
type _ int
b[blen-1] = '"' // Found IsInBounds
b2 = b[:blen] // Found IsSliceInBounds
}
func main() {
b[blen-1] = '"' // Found IsInBounds
b2 = b[:blen] // Found IsSliceInBounds
ggg()
ggg2(b)
ggg3(blen)
fff(b, blen)
} |
@go101, the compiler assumes global variables can be reassigned at any time. The two reads of |
@randall77, then it looks this issue could be closed now, as this problem doesn't happen to local slices. BTW, more evidences to prove global variables are not BCE friendly. package main
var blen int = 1
var b []byte = make([]byte, 1)
var b2 []byte
var x byte
func ggg1() {
_ = b[blen-1] // Found IsInBounds
b2 = b[:blen]
}
func ggg2() {
_ = b[blen-1] // Found IsInBounds
b2 = b[:blen]
b[blen-1] = '"' // Found IsInBounds
}
func ggg3() {
_ = b[blen-1] // Found IsInBounds
b[blen-1] = '"'
b2 = b[:blen] // Found IsSliceInBounds
}
func ggg1b() {
x = b[blen-1] // Found IsInBounds
b2 = b[:blen] // Found IsSliceInBounds
}
func ggg2b() {
x = b[blen-1] // Found IsInBounds
b2 = b[:blen] // Found IsSliceInBounds
b[blen-1] = '"' // Found IsInBounds
}
func ggg3b() {
x = b[blen-1] // Found IsInBounds
b[blen-1] = '"' // Found IsInBounds
b2 = b[:blen] // Found IsSliceInBounds
}
func main() {
ggg1()
ggg2()
ggg3()
} |
It looks there is still a little unreasonable in BCE decision making: package main
var s = make([]int, 100)
func f() (r int) {
for i := range s {
r += s[i] // bound check eliminated
}
return r
}
func g() {
for i := range s {
s[i] = i // need bound check
}
}
func main() {
} I filed another issue: #47736 |
I expect that if we can access the nth element of a slice, then we can slice up to n+1.
See below.
Consequently, I do not expect a bounds check for b[:blen].
However, I get a bounds check for both b[blen-1] and b[:blen].
What version of Go are you using (
go version
)?Does this issue reproduce with the latest release?
Yes
What operating system and processor architecture are you using (
go env
)?go env
OutputWhat did you do?
What did you expect to see?
What did you see instead?
The text was updated successfully, but these errors were encountered: