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: 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

Comments

@rillig
Copy link
Contributor

@rillig rillig commented Mar 7, 2020

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

@randall77 randall77 commented Mar 8, 2020

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 rasky commented Mar 8, 2020

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 mvdan commented Mar 8, 2020

Also CC @zdjones

@rillig
Copy link
Contributor Author

@rillig rillig commented Mar 8, 2020

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
5 participants
You can’t perform that action at this time.