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: avoid unnecessary calls to runtime.panicIndex #37740

Open
rillig opened this issue Mar 7, 2020 · 4 comments
Open

cmd/compile: avoid unnecessary calls to runtime.panicIndex #37740

rillig opened this issue Mar 7, 2020 · 4 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
Unplanned

Comments

@rillig
Copy link
Contributor

rillig commented Mar 7, 2020 •
edited

What version of Go are you using?

go version go1.14 windows/amd64

What did you do?

https://play.golang.org/p/uFSviGB2UL3

What did you expect to see?

The generated machine code contains zero calls to runtime.panicIndex since all index calculations are safe, and this can be proven easily.

As a bonus, the code generator should recognize that the pattern parts[n - x] appears repeatedly with varying small constants for x. There is no need to calculate the expression 16 * n repeatedly in lines 30 to 33.

What did you see instead?

go tool objdump -s joinCambridge contains 7 calls to runtime.panicIndex.
@randall77 randall77 added this to the Unplanned milestone Mar 8, 2020
@randall77
Copy link
Contributor

This is the general problem of bounds check elimination.
We'll keep this as another example that needs work.

@brtzsnr @rasky

@rasky
Copy link
Member

rasky commented Mar 8, 2020 •
edited

I don’t think the request is correct. 2+2*len(elements) might overflow, so the bound checks are actually required.

@mvdan
Copy link
Member

mvdan commented Mar 8, 2020

Also CC @zdjones

@rillig
Copy link
Contributor Author

rillig commented Mar 8, 2020 •
edited

I don’t think the request is correct. 2+2*len(elements) might overflow, so the bound checks are actually required.

Damn, I didn't think of that. On a 32-bit platform an overflow is entirely possible. On a 64-bit platform with enough virtual memory, you would only need 67 million bars of 256 GB RAM each, which is expensive but not impossible.

My rescue argument to this is: In this particular case, the size of each slice entry takes 2 machine words (it's a string, consisting of start address and length). Since len(elements) == sizeof(elements) / sizeof(elements[0]) and sizeof(elements[0]) is at least 8, the expression len(elements) cannot overflow. Phew.

@toothrot toothrot added the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Mar 9, 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
6 participants
@toothrot @rasky @rillig @mvdan @randall77 @gopherbot