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: funccompile is reentrant #33485

Closed
mdempsky opened this issue Aug 5, 2019 · 4 comments
Closed

cmd/compile: funccompile is reentrant #33485

mdempsky opened this issue Aug 5, 2019 · 4 comments
Labels
FrozenDueToAge NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Milestone

Comments

@mdempsky
Copy link
Member

mdempsky commented Aug 5, 2019

There's an awkward reentrancy issue with funccompile: funccompile calls compile, which calls dtypesym, which calls geneq/genhash/genwrapper, which call back into funccompile.

I don't think this is causing any problems today, but I was trying to work on #17728, and it led to some confusing issues due to SSA cache corruption.

It would probably be better if generated ASTs were pumped through the same compile workqueue that normal ASTs are compiled.

@mdempsky mdempsky added the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Aug 5, 2019
@mdempsky mdempsky added this to the Unplanned milestone Aug 5, 2019
@gopherbot
Copy link

Change https://golang.org/cl/254617 mentions this issue: cmd/compile: make funccompile un-reentrant

@gopherbot
Copy link

Change https://golang.org/cl/254839 mentions this issue: cmd/compile: call fninit earlier

@gopherbot
Copy link

Change https://golang.org/cl/254838 mentions this issue: cmd/compile: better dclcontext handling in func{hdr,body}

gopherbot pushed a commit that referenced this issue Sep 14, 2020
funchdr and funcbody currently assume that either (1) Curfn == nil &&
dclcontext == PEXTERN, or (2) Curfn != nil && dclcontext == PAUTO.
This is a reasonable assumption during parsing. However, these
functions end up getting used in other contexts, and not all callers
are so disciplined about Curfn/dclcontext handling.

This CL changes them to save/restore arbitrary Curfn/dclcontext pairs
instead. This is necessary for the followup CL, which pushes fninit
earlier. Otherwise, Curfn/dclcontext fall out of sync, and funchdr
panics.

Passes toolstash-check.

Updates #33485.

Change-Id: I19b1be23db1bad6475345ae5c81bbdc66291a3a7
Reviewed-on: https://go-review.googlesource.com/c/go/+/254838
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Keith Randall <khr@golang.org>
Trust: Matthew Dempsky <mdempsky@google.com>
gopherbot pushed a commit that referenced this issue Sep 14, 2020
This allows the global initializers function to go through normal
mid-end optimizations (e.g., inlining, escape analysis) like any other
function.

Updates #33485.

Change-Id: I9bcfe98b8628d1aca09b4c238d8d3b74c69010a5
Reviewed-on: https://go-review.googlesource.com/c/go/+/254839
Reviewed-by: Keith Randall <khr@golang.org>
Trust: Matthew Dempsky <mdempsky@google.com>
@gopherbot
Copy link

Change https://golang.org/cl/254938 mentions this issue: test: fix inline.go to pass linux-amd64-noopt

gopherbot pushed a commit that referenced this issue Sep 15, 2020
Updates #33485.

Change-Id: I3330860cdff1e9797466a7630bcdb7792c465b06
Reviewed-on: https://go-review.googlesource.com/c/go/+/254938
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
Reviewed-by: Cuong Manh Le <cuong.manhle.vn@gmail.com>
TryBot-Result: Go Bot <gobot@golang.org>
Trust: Matthew Dempsky <mdempsky@google.com>
@golang golang locked and limited conversation to collaborators Sep 15, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
FrozenDueToAge NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Projects
None yet
Development

No branches or pull requests

2 participants