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: possible escape analysis cleanup #4315

Closed
rsc opened this issue Oct 30, 2012 · 10 comments
Closed

cmd/compile: possible escape analysis cleanup #4315

rsc opened this issue Oct 30, 2012 · 10 comments

Comments

@rsc
Copy link
Contributor

rsc commented Oct 30, 2012

from lvd; not a requirement for go 1.1

diff -r 959dee37d03d src/cmd/gc/esc.c
--- a/src/cmd/gc/esc.c Tue Oct 30 11:10:08 2012 +1100
+++ b/src/cmd/gc/esc.c Tue Oct 30 11:24:15 2012 +0100
@@ -390,15 +390,18 @@
  break;
  }

- escloopdepth(e, n->left);
- escloopdepth(e, n->right);
- escloopdepthlist(e, n->list);
- escloopdepth(e, n->ntest);
+ // TODO(lvd) left, right, list, ntest and rlist should only contain
expressions, not statements
+ // and the standard tree compiles fine with these branches commented
out, but that may not
+ // outweigh the time spent debugging a misjudged label leading to a
spurious stackallocation
+ // when someone violates this assumption somewhere unrelated.
+// escloopdepth(e, n->left);
+// escloopdepth(e, n->right);
+// escloopdepthlist(e, n->list);
+// escloopdepth(e, n->ntest);
  escloopdepth(e, n->nincr);
  escloopdepthlist(e, n->nbody);
  escloopdepthlist(e, n->nelse);
- escloopdepthlist(e, n->rlist);
-
+// escloopdepthlist(e, n->rlist);
 }

 static void
@rsc
Copy link
Contributor Author

rsc commented Jul 30, 2013

Comment 1:

Labels changed: added go1.2maybe.

@rsc
Copy link
Contributor Author

rsc commented Jul 30, 2013

Comment 2:

Labels changed: added feature.

@robpike
Copy link
Contributor

robpike commented Aug 29, 2013

Comment 3:

Not for 1.2.

Labels changed: removed go1.2maybe.

@rsc
Copy link
Contributor Author

rsc commented Nov 27, 2013

Comment 4:

Labels changed: added go1.3maybe.

@rsc
Copy link
Contributor Author

rsc commented Nov 27, 2013

Comment 5:

Labels changed: removed feature.

@rsc
Copy link
Contributor Author

rsc commented Dec 4, 2013

Comment 6:

Labels changed: added release-none, removed go1.3maybe.

@rsc
Copy link
Contributor Author

rsc commented Dec 4, 2013

Comment 7:

Labels changed: added repo-main.

@rsc rsc added this to the Unplanned milestone Apr 10, 2015
@rsc rsc changed the title cmd/gc: possible escape analysis cleanup cmd/compile: possible escape analysis cleanup Jun 8, 2015
@odeke-em
Copy link
Member

@randall77 do you think we should close this issue since the C code being referenced is no longer even in the source?

@randall77
Copy link
Contributor

Yes, I think this issue is outdated. @dr2chase rewrote a lot of the escape analysis code.
Closing. If David thinks it is still relevant, he can reopen.

@dr2chase
Copy link
Contributor

I'm happy leaving this closed, although(because) I am still not happy with the escape analysis code.

@golang golang locked and limited conversation to collaborators Sep 26, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

6 participants