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

cmd/compile: prove should be taught about the bounds of (i+j)/2 #54544

Open
mvdan opened this issue Aug 19, 2022 · 1 comment
Open

cmd/compile: prove should be taught about the bounds of (i+j)/2 #54544

mvdan opened this issue Aug 19, 2022 · 1 comment
Labels
compiler/runtime Issues related to the Go compiler and/or runtime. NeedsFix The path to resolution is known, but the work has not been done. Performance
Milestone

Comments

@mvdan
Copy link
Member

mvdan commented Aug 19, 2022

$ go version
go version devel go1.20-0a4a57de4d Thu Aug 18 21:54:52 2022 +0000 linux/amd64

sort.Search, and its inlined copy in go/token.searchInts, both have code like:

func searchInts(a []int, x int) int {
    i, j := 0, len(a)    
    for i < j {
        h := int(uint(i+j) >> 1) // avoid overflow when computing h
        // i ≤ h < j  
        if a[h] <= x { // Found IsInBounds
            i = h + 1
        } else {
            j = h
        }
    }
    return i - 1   
}

Note that a[h] causes a bounds check, as the compiler doesn't know that 0 <= h < len(a). We intuitively know that, because h is the middle point between i and j, thus i <= h <= j. i and j themselves start as 0 and len(a) and move closer towards h.

It would be nice if the prove pass could be smart enough to tell that a[h] here does not need a bounds check. I can force its hand via if h >= 0 && h < len(a) && a[h] <= x {, but that doesn't result in a noticeable improvement:

name           old time/op  new time/op  delta
SearchInts-16  16.2ns ± 2%  16.5ns ± 1%  +1.50%  (p=0.001 n=8+10)

I realize that this request is fairly specific to binary searches, but I reckon that's a common enough occurrence that we should teach prove about it.

cc @egonelbre @golang/compiler

@seankhliao seankhliao changed the title cmd/go: prove should be taught about the bounds of (i+j)/2 cmd/compile: prove should be taught about the bounds of (i+j)/2 Aug 19, 2022
@gopherbot gopherbot added the compiler/runtime Issues related to the Go compiler and/or runtime. label Aug 19, 2022
@mvdan
Copy link
Member Author

mvdan commented Aug 19, 2022

I slightly got the bounds wrong, because I assumed that i <= j. The bounds for h = (i+j)/2 are instead min(i, j) <= h <= max(i, j), which should still be enough given that our i and j never become negative or larger than len(a).

@randall77 randall77 added this to the Unplanned milestone Aug 19, 2022
@joedian joedian added the NeedsFix The path to resolution is known, but the work has not been done. label Aug 19, 2022
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. NeedsFix The path to resolution is known, but the work has not been done. Performance
Projects
None yet
Development

No branches or pull requests

4 participants