Skip to content

Commit 3692925

Browse files
committed
cmd/compile: move Text.From.Sym initialization earlier
The initialization of an ATEXT Prog's From.Sym can race with the assemblers in a concurrent compiler. CL 40254 contains an initial, failed attempt to fix that race. This CL takes a different approach: Rather than expose an API to initialize the Prog, expose an API to initialize the Sym. The initialization of the Sym can then be moved earlier in the compiler, avoiding the race. The growth of gc.Func has negligible performance impact; see below. Passes toolstash -cmp. Updates #15756 name old alloc/op new alloc/op delta Template 38.8MB ± 0% 38.8MB ± 0% ~ (p=0.968 n=9+10) Unicode 29.8MB ± 0% 29.8MB ± 0% ~ (p=0.684 n=10+10) GoTypes 113MB ± 0% 113MB ± 0% ~ (p=0.912 n=10+10) SSA 1.25GB ± 0% 1.25GB ± 0% ~ (p=0.481 n=10+10) Flate 25.3MB ± 0% 25.3MB ± 0% ~ (p=0.105 n=10+10) GoParser 31.7MB ± 0% 31.8MB ± 0% +0.09% (p=0.016 n=8+10) Reflect 78.3MB ± 0% 78.2MB ± 0% ~ (p=0.190 n=10+10) Tar 26.5MB ± 0% 26.6MB ± 0% +0.13% (p=0.011 n=10+10) XML 42.4MB ± 0% 42.4MB ± 0% ~ (p=0.971 n=10+10) name old allocs/op new allocs/op delta Template 378k ± 1% 378k ± 0% ~ (p=0.315 n=10+9) Unicode 321k ± 1% 321k ± 0% ~ (p=0.436 n=10+10) GoTypes 1.14M ± 0% 1.14M ± 0% ~ (p=0.079 n=10+9) SSA 9.70M ± 0% 9.70M ± 0% -0.04% (p=0.035 n=10+10) Flate 233k ± 1% 234k ± 1% ~ (p=0.529 n=10+10) GoParser 315k ± 0% 316k ± 0% ~ (p=0.095 n=9+10) Reflect 980k ± 0% 980k ± 0% ~ (p=0.436 n=10+10) Tar 249k ± 1% 250k ± 0% ~ (p=0.280 n=10+10) XML 391k ± 1% 391k ± 1% ~ (p=0.481 n=10+10) Change-Id: I3c93033dddd2e1df8cc54a106a6e615d27859e71 Reviewed-on: https://go-review.googlesource.com/40496 Run-TryBot: Josh Bleecher Snyder <[email protected]> TryBot-Result: Gobot Gobot <[email protected]> Reviewed-by: Cherry Zhang <[email protected]>
1 parent 7faf302 commit 3692925

File tree

6 files changed

+33
-24
lines changed

6 files changed

+33
-24
lines changed

src/cmd/asm/internal/asm/asm.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -160,6 +160,7 @@ func (p *Parser) asmText(word string, operands [][]lex.Token) {
160160
}
161161
argSize = p.positiveAtoi(op[1].String())
162162
}
163+
p.ctxt.InitTextSym(nameAddr.Sym, int(flag))
163164
prog := &obj.Prog{
164165
Ctxt: p.ctxt,
165166
As: obj.ATEXT,
@@ -171,9 +172,8 @@ func (p *Parser) asmText(word string, operands [][]lex.Token) {
171172
// Argsize set below.
172173
},
173174
}
175+
nameAddr.Sym.Text = prog
174176
prog.To.Val = int32(argSize)
175-
p.ctxt.InitTextSym(prog, int(flag))
176-
177177
p.append(prog, "", true)
178178
}
179179

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

Lines changed: 24 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -144,50 +144,59 @@ func (pp *Progs) settext(fn *Node) {
144144
if pp.Text != nil {
145145
Fatalf("Progs.settext called twice")
146146
}
147-
148147
ptxt := pp.Prog(obj.ATEXT)
149-
if nam := fn.Func.Nname; !isblank(nam) {
148+
if fn.Func.lsym != nil {
149+
fn.Func.lsym.Text = ptxt
150150
ptxt.From.Type = obj.TYPE_MEM
151151
ptxt.From.Name = obj.NAME_EXTERN
152-
ptxt.From.Sym = Linksym(nam.Sym)
153-
if fn.Func.Pragma&Systemstack != 0 {
154-
ptxt.From.Sym.Set(obj.AttrCFunc, true)
152+
ptxt.From.Sym = fn.Func.lsym
153+
}
154+
pp.Text = ptxt
155+
}
156+
157+
func (f *Func) initLSym() {
158+
if f.lsym != nil {
159+
Fatalf("Func.initLSym called twice")
160+
}
161+
162+
if nam := f.Nname; !isblank(nam) {
163+
f.lsym = Linksym(nam.Sym)
164+
if f.Pragma&Systemstack != 0 {
165+
f.lsym.Set(obj.AttrCFunc, true)
155166
}
156167
}
157168

158169
var flag int
159-
if fn.Func.Dupok() {
170+
if f.Dupok() {
160171
flag |= obj.DUPOK
161172
}
162-
if fn.Func.Wrapper() {
173+
if f.Wrapper() {
163174
flag |= obj.WRAPPER
164175
}
165-
if fn.Func.NoFramePointer() {
176+
if f.NoFramePointer() {
166177
flag |= obj.NOFRAME
167178
}
168-
if fn.Func.Needctxt() {
179+
if f.Needctxt() {
169180
flag |= obj.NEEDCTXT
170181
}
171-
if fn.Func.Pragma&Nosplit != 0 {
182+
if f.Pragma&Nosplit != 0 {
172183
flag |= obj.NOSPLIT
173184
}
174-
if fn.Func.ReflectMethod() {
185+
if f.ReflectMethod() {
175186
flag |= obj.REFLECTMETHOD
176187
}
177188

178189
// Clumsy but important.
179190
// See test/recover.go for test cases and src/reflect/value.go
180191
// for the actual functions being considered.
181192
if myimportpath == "reflect" {
182-
switch fn.Func.Nname.Sym.Name {
193+
switch f.Nname.Sym.Name {
183194
case "callReflect", "callMethod":
184195
flag |= obj.WRAPPER
185196
}
186197
}
187198

188-
Ctxt.InitTextSym(ptxt, flag)
189-
190-
pp.Text = ptxt
199+
Ctxt.InitTextSym(f.lsym, flag)
191200
}
192201

193202
func ggloblnod(nam *Node) {

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

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -297,6 +297,9 @@ func compile(fn *Node) {
297297
// From this point, there should be no uses of Curfn. Enforce that.
298298
Curfn = nil
299299

300+
// Set up the function's LSym early to avoid data races with the assemblers.
301+
fn.Func.initLSym()
302+
300303
// Build an SSA backend function.
301304
ssafn := buildssa(fn)
302305
pp := newProgs(fn)

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,7 @@ func TestSizeof(t *testing.T) {
2222
_32bit uintptr // size on 32bit platforms
2323
_64bit uintptr // size on 64bit platforms
2424
}{
25-
{Func{}, 96, 160},
25+
{Func{}, 100, 168},
2626
{Name{}, 36, 56},
2727
{Param{}, 28, 56},
2828
{Node{}, 84, 136},

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

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ package gc
99
import (
1010
"cmd/compile/internal/syntax"
1111
"cmd/compile/internal/types"
12+
"cmd/internal/obj"
1213
"cmd/internal/src"
1314
)
1415

@@ -331,6 +332,7 @@ type Func struct {
331332
Top int // top context (Ecall, Eproc, etc)
332333
Closure *Node // OCLOSURE <-> ODCLFUNC
333334
Nname *Node
335+
lsym *obj.LSym
334336

335337
Inl Nodes // copy of the body for use in inlining
336338
InlCost int32

src/cmd/internal/obj/plist.go

Lines changed: 1 addition & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -112,11 +112,7 @@ func Flushplist(ctxt *Link, plist *Plist, newprog ProgAlloc) {
112112
ctxt.Text = append(ctxt.Text, text...)
113113
}
114114

115-
func (ctxt *Link) InitTextSym(p *Prog, flag int) {
116-
if p.As != ATEXT {
117-
ctxt.Diag("InitTextSym non-ATEXT: %v", p)
118-
}
119-
s := p.From.Sym
115+
func (ctxt *Link) InitTextSym(s *LSym, flag int) {
120116
if s == nil {
121117
// func _() { }
122118
return
@@ -139,7 +135,6 @@ func (ctxt *Link) InitTextSym(p *Prog, flag int) {
139135
s.Set(AttrNeedCtxt, flag&NEEDCTXT != 0)
140136
s.Set(AttrNoFrame, flag&NOFRAME != 0)
141137
s.Type = STEXT
142-
s.Text = p
143138
}
144139

145140
func (ctxt *Link) Globl(s *LSym, size int64, flag int) {

0 commit comments

Comments
 (0)