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.

Sign up for GitHub

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: Renumbering Value IDs before register allocation can save time/space #15652

Closed
dr2chase opened this issue May 11, 2016 · 3 comments
Closed

cmd/compile: Renumbering Value IDs before register allocation can save time/space #15652

dr2chase opened this issue May 11, 2016 · 3 comments
Labels
FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done.
Milestone
Go1.8Early

Comments

@dr2chase
Copy link
Contributor

Some large inputs (see #15537) create a large number of ssa values early in the compilation, but most of those disappear early (because of excess LocalRef/Copy/Phi creation that is optimized out). Func.NumValues() however continues to return the high-water-mark for value ID, which leads to overallocation of various intermediate data structures. This is especially costly in register allocation.

Renumbering the range of Value Ids to compress the range saves time and space.

The benefits of renumbering are reduced by other methods for reducing unnecessary Value creation; eliminating Call blocks is one way (see #15631 for this), sparse methods for finding/creating phi functions is another (see https://go-review.googlesource.com/#/c/22342/ ). Renumbering will interfere with tracking values in the ssa.html debugging output; however, for the inputs where renumbering produces a noticeable improvement the ssa.html files are so large as to be unwieldy (gigabytes).
#15631 contains an early discussion of the motivation for this bug/fix.
@dr2chase dr2chase added this to the Go1.8Early milestone May 11, 2016
@gopherbot
Copy link

CL https://golang.org/cl/23042 mentions this issue.

@quentinmit quentinmit added the NeedsFix The path to resolution is known, but the work has not been done. label Oct 6, 2016
@rsc
Copy link
Contributor

rsc commented Oct 17, 2016

The comment on the abandoning of the CL makes it sound like we decided this was not something we wanted to do after all.

@rsc rsc closed this as completed Oct 17, 2016
@dr2chase
Copy link
Contributor Author

The desirability depends almost entirely on whether or not any particular phase creates a zillion value nodes that then go dead, because the maximum value index is what determines the memory footprint of some operations. Right now that seems not to be happening, so we're okay without it.

AND, if you do this, and you need to debug the SSA code, tracing values between phases gets much harder.

@golang golang locked and limited conversation to collaborators Oct 17, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done.
Projects
None yet
Milestone
Go1.8Early
Development

No branches or pull requests
4 participants
@rsc @quentinmit @dr2chase @gopherbot