Skip to content

Commit d20298e

Browse files
committed
cmd/compile: make funccompile non-reentrant
Currently, there's awkward reentrancy issue with funccompile: funccompile -> compile -> dtypesym -> geneq/genhash/genwrapper -> funccompile Though it's not a problem at this moment, some attempts by @mdempsky to move order/walk/instrument into buildssa was failed, due to SSA cache corruption. This commit fixes that reentrancy issue, by making generated functions to be pumped through the same compile workqueue that normal functions are compiled. We do this by adding them to xtop, instead of calling funccompile directly in geneq/genhash/genwrapper. In dumpdata, we look for uncompiled functions in xtop instead of compilequeue, then finish compiling them. Updates #38463 Fixes #33485 Change-Id: Ic9f0ce45b56ae2ff3862f17fd979253ddc144bb5 Reviewed-on: https://go-review.googlesource.com/c/go/+/254617 Run-TryBot: Cuong Manh Le <[email protected]> Reviewed-by: Matthew Dempsky <[email protected]> TryBot-Result: Go Bot <[email protected]> Trust: Keith Randall <[email protected]> Trust: Cuong Manh Le <[email protected]>
1 parent f4936d0 commit d20298e

File tree

3 files changed

+28
-4
lines changed

3 files changed

+28
-4
lines changed

src/cmd/compile/internal/gc/alg.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -392,7 +392,7 @@ func genhash(t *types.Type) *obj.LSym {
392392
}
393393

394394
fn.Func.SetNilCheckDisabled(true)
395-
funccompile(fn)
395+
xtop = append(xtop, fn)
396396

397397
// Build closure. It doesn't close over any variables, so
398398
// it contains just the function pointer.
@@ -754,7 +754,7 @@ func geneq(t *types.Type) *obj.LSym {
754754
// neither of which can be nil, and our comparisons
755755
// are shallow.
756756
fn.Func.SetNilCheckDisabled(true)
757-
funccompile(fn)
757+
xtop = append(xtop, fn)
758758

759759
// Generate a closure which points at the function we just generated.
760760
dsymptr(closure, 0, sym.Linksym(), 0)

src/cmd/compile/internal/gc/obj.go

Lines changed: 25 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -113,12 +113,16 @@ func dumpCompilerObj(bout *bio.Writer) {
113113

114114
func dumpdata() {
115115
externs := len(externdcl)
116+
xtops := len(xtop)
116117

117118
dumpglobls()
118119
addptabs()
120+
exportlistLen := len(exportlist)
119121
addsignats(externdcl)
120122
dumpsignats()
121123
dumptabs()
124+
ptabsLen := len(ptabs)
125+
itabsLen := len(itabs)
122126
dumpimportstrings()
123127
dumpbasictypes()
124128

@@ -129,9 +133,19 @@ func dumpdata() {
129133
// number of types in a finite amount of code.
130134
// In the typical case, we loop 0 or 1 times.
131135
// It was not until issue 24761 that we found any code that required a loop at all.
132-
for len(compilequeue) > 0 {
136+
for {
137+
for i := xtops; i < len(xtop); i++ {
138+
n := xtop[i]
139+
if n.Op == ODCLFUNC {
140+
funccompile(n)
141+
}
142+
}
143+
xtops = len(xtop)
133144
compileFunctions()
134145
dumpsignats()
146+
if xtops == len(xtop) {
147+
break
148+
}
135149
}
136150

137151
// Dump extra globals.
@@ -149,6 +163,16 @@ func dumpdata() {
149163
}
150164

151165
addGCLocals()
166+
167+
if exportlistLen != len(exportlist) {
168+
Fatalf("exportlist changed after compile functions loop")
169+
}
170+
if ptabsLen != len(ptabs) {
171+
Fatalf("ptabs changed after compile functions loop")
172+
}
173+
if itabsLen != len(itabs) {
174+
Fatalf("itabs changed after compile functions loop")
175+
}
152176
}
153177

154178
func dumpLinkerObj(bout *bio.Writer) {

src/cmd/compile/internal/gc/subr.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1615,7 +1615,7 @@ func genwrapper(rcvr *types.Type, method *types.Field, newnam *types.Sym) {
16151615
escapeFuncs([]*Node{fn}, false)
16161616

16171617
Curfn = nil
1618-
funccompile(fn)
1618+
xtop = append(xtop, fn)
16191619
}
16201620

16211621
func paramNnames(ft *types.Type) []*Node {

0 commit comments

Comments
 (0)