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: revisit append codegen #24209

Open
josharian opened this issue Mar 2, 2018 · 3 comments
Open

cmd/compile: revisit append codegen #24209

josharian opened this issue Mar 2, 2018 · 3 comments
Labels
Milestone

Comments

@josharian
Copy link
Contributor

@josharian josharian commented Mar 2, 2018

CLs 21813 and 22197 optimized the code generated for append. It was done in part to address #14921, but while I was there, I also focused on things like avoiding spills in the fast path.

Several things have changed in the compiler since then:

  • The old backend is gone, so we are free to change the signature of growslice.
  • We are better at sinking spills to a narrower scope, so some of the spill-avoidance techniques used are no longer necessary.
  • Write barriers are inserted later in the SSA backend, rather than in the frontend and during SSA construction.

All this suggests to me that we may be able to simplify and improve the code generated for append.

I also suspect that the "no write barriers for in-place appends" optimization may have gotten lost during the write barrier move. More details about that in a comment-to-come over in #14921.

That's a significant optimization. Tentatively marking this as a release blocker for that reason.

cc @randall77 @cherrymui @aclements @mvdan

@josharian
Copy link
Contributor Author

@josharian josharian commented May 6, 2018

I also suspect that the "no write barriers for in-place appends" optimization may have gotten lost during the write barrier move.

I was wrong. Therefore, this is not a release blocker. Punting to Go 1.12.

@randall77
Copy link
Contributor

@randall77 randall77 commented Dec 12, 2018

Punting to 1.13, too late for anything major in 1.12.

@randall77 randall77 modified the milestones: Go1.12, Go1.13 Dec 12, 2018
@agnivade

This comment was marked as outdated.

@agnivade agnivade modified the milestones: Go1.13, Go1.14 Jun 2, 2019
@rsc rsc modified the milestones: Go1.14, Backlog Oct 9, 2019
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
4 participants
You can’t perform that action at this time.