cmd/internal/obj: rework gclocals handling

The compiler handled gcargs and gclocals LSyms unusually.
It generated placeholder symbols (makefuncdatasym),
filled them in, and then renamed them for content-addressability.
This is an important binary size optimization;
the same locals information occurs over and over.

This CL continues to treat these LSyms unusually,
but in a slightly more explicit way,
and importantly for concurrent compilation,
in a way that does not require concurrent
modification of Ctxt.Hash.

Instead of creating gcargs and gclocals in the usual way,
by creating a types.Sym and then an obj.LSym,
we add them directly to obj.FuncInfo,
initialize them in obj.InitTextSym,
and deduplicate and add them to ctxt.Data at the end.
Then the backend's job is simply to fill them in
and rename them appropriately.

Updates #15756

name       old alloc/op      new alloc/op      delta
Template        38.8MB ± 0%       38.7MB ± 0%  -0.22%  (p=0.016 n=5+5)
Unicode         29.8MB ± 0%       29.8MB ± 0%    ~     (p=0.690 n=5+5)
GoTypes          113MB ± 0%        113MB ± 0%  -0.24%  (p=0.008 n=5+5)
SSA             1.25GB ± 0%       1.24GB ± 0%  -0.39%  (p=0.008 n=5+5)
Flate           25.3MB ± 0%       25.2MB ± 0%  -0.43%  (p=0.008 n=5+5)
GoParser        31.7MB ± 0%       31.7MB ± 0%  -0.22%  (p=0.008 n=5+5)
Reflect         78.2MB ± 0%       77.6MB ± 0%  -0.80%  (p=0.008 n=5+5)
Tar             26.6MB ± 0%       26.3MB ± 0%  -0.85%  (p=0.008 n=5+5)
XML             42.4MB ± 0%       41.9MB ± 0%  -1.04%  (p=0.008 n=5+5)

name       old allocs/op     new allocs/op     delta
Template          378k ± 0%         377k ± 1%    ~     (p=0.151 n=5+5)
Unicode           321k ± 1%         321k ± 0%    ~     (p=0.841 n=5+5)
GoTypes          1.14M ± 0%        1.14M ± 0%  -0.47%  (p=0.016 n=5+5)
SSA              9.71M ± 0%        9.67M ± 0%  -0.33%  (p=0.008 n=5+5)
Flate             233k ± 1%         232k ± 1%    ~     (p=0.151 n=5+5)
GoParser          316k ± 0%         315k ± 0%  -0.49%  (p=0.016 n=5+5)
Reflect           979k ± 0%         972k ± 0%  -0.75%  (p=0.008 n=5+5)
Tar               250k ± 0%         247k ± 1%  -0.92%  (p=0.008 n=5+5)
XML               392k ± 1%         389k ± 0%  -0.67%  (p=0.008 n=5+5)

Change-Id: Idc36186ca9d2f8214b5f7720bbc27b6bb22fdc48
Reviewed-on: https://go-review.googlesource.com/40697
Run-TryBot: Josh Bleecher Snyder <josharian@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>

src/cmd/compile/fmt_test.go

@@ -654,7 +654,6 @@ var knownFormats = map[string]string{
 	"cmd/compile/internal/types.EType %v":             "",
 	"cmd/internal/src.Pos %s":                         "",
 	"cmd/internal/src.Pos %v":                         "",
-	"cmd/internal/src.XPos %v":                        "",
 	"error %v":                                        "",
 	"float64 %.2f":                                    "",
 	"float64 %.3f":                                    "",

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

@@ -145,13 +145,29 @@ func (pp *Progs) settext(fn *Node) {
 		Fatalf("Progs.settext called twice")
 	}
 	ptxt := pp.Prog(obj.ATEXT)
-	if fn.Func.lsym != nil {
+	pp.Text = ptxt
+
+	if fn.Func.lsym == nil {
+		// func _() { }
+		return
+	}
+
 	fn.Func.lsym.Text = ptxt
 	ptxt.From.Type = obj.TYPE_MEM
 	ptxt.From.Name = obj.NAME_EXTERN
 	ptxt.From.Sym = fn.Func.lsym
-	}
-	pp.Text = ptxt
+
+	p := pp.Prog(obj.AFUNCDATA)
+	Addrconst(&p.From, obj.FUNCDATA_ArgsPointerMaps)
+	p.To.Type = obj.TYPE_MEM
+	p.To.Name = obj.NAME_EXTERN
+	p.To.Sym = &fn.Func.lsym.FuncInfo.GCArgs
+
+	p = pp.Prog(obj.AFUNCDATA)
+	Addrconst(&p.From, obj.FUNCDATA_LocalsPointerMaps)
+	p.To.Type = obj.TYPE_MEM
+	p.To.Name = obj.NAME_EXTERN
+	p.To.Sym = &fn.Func.lsym.FuncInfo.GCLocals
 }
 
 func (f *Func) initLSym() {

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

@@ -152,6 +152,8 @@ func dumpobj1(outfile string, mode int) {
 		ggloblsym(zero, int32(zerosize), obj.DUPOK|obj.RODATA)
 	}
 
+	addGCLocals()
+
 	obj.WriteObjFile(Ctxt, bout.Writer)
 
 	if writearchive {
@@ -227,6 +229,27 @@ func dumpglobls() {
 	funcsyms = nil
 }
 
+// addGCLocals adds gcargs and gclocals symbols to Ctxt.Data.
+// It takes care not to add any duplicates.
+// Though the object file format handles duplicates efficiently,
+// storing only a single copy of the data,
+// failure to remove these duplicates adds a few percent to object file size.
+func addGCLocals() {
+	seen := make(map[string]bool)
+	for _, s := range Ctxt.Text {
+		if s.FuncInfo == nil {
+			continue
+		}
+		for _, gcsym := range []*obj.LSym{&s.FuncInfo.GCArgs, &s.FuncInfo.GCLocals} {
+			if seen[gcsym.Name] {
+				continue
+			}
+			Ctxt.Data = append(Ctxt.Data, gcsym)
+			seen[gcsym.Name] = true
+		}
+	}
+}
+
 func linksymname(s *types.Sym) string {
 	if isblanksym(s) {
 		return "_"
@@ -279,11 +302,19 @@ func duintptr(s *types.Sym, off int, v uint64) int {
 	return duintxx(s, off, v, Widthptr)
 }
 
-func dbvec(s *types.Sym, off int, bv bvec) int {
+func duint8LSym(s *obj.LSym, off int, v uint8) int {
+	return duintxxLSym(s, off, uint64(v), 1)
+}
+
+func duint32LSym(s *obj.LSym, off int, v uint32) int {
+	return duintxxLSym(s, off, uint64(v), 4)
+}
+
+func dbvecLSym(s *obj.LSym, off int, bv bvec) int {
 	// Runtime reads the bitmaps as byte arrays. Oblige.
 	for j := 0; int32(j) < bv.n; j += 8 {
 		word := bv.b[j/32]
-		off = duint8(s, off, uint8(word>>(uint(j)%32)))
+		off = duint8LSym(s, off, uint8(word>>(uint(j)%32)))
 	}
 	return off
 }

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

@@ -17,39 +17,6 @@ import (
 
 // "Portable" code generation.
 
-func makefuncdatasym(pp *Progs, nameprefix string, funcdatakind int64, curfn *Node) *types.Sym {
-	// This symbol requires a unique, reproducible name;
-	// unique to avoid duplicate symbols,
-	// and reproducible for reproducible builds and toolstash.
-	// The function name will usually suffice.
-	suffix := curfn.Func.Nname.Sym.Name
-	if suffix == "_" {
-		// It is possible to have multiple functions called _,
-		// so in this rare case, use instead the function's position.
-		// This formatted string will be meaningless gibberish, but that's ok.
-		// It will be unique and reproducible, and it is rare anyway.
-		// Note that we can't just always use the position;
-		// it is possible to have multiple autogenerated functions at the same position.
-		// Fortunately, no autogenerated functions are called _.
-		if curfn.Pos == autogeneratedPos {
-			Fatalf("autogenerated func _")
-		}
-		suffix = fmt.Sprintf("%v", curfn.Pos)
-	}
-	// Add in the package path as well.
-	// When generating wrappers, we can end up compiling a function belonging
-	// to other packages, which might have a name that collides with one in our package.
-	symname := nameprefix + curfn.Func.Nname.Sym.Pkg.Path + "." + suffix
-
-	sym := lookup(symname)
-	p := pp.Prog(obj.AFUNCDATA)
-	Addrconst(&p.From, funcdatakind)
-	p.To.Type = obj.TYPE_MEM
-	p.To.Name = obj.NAME_EXTERN
-	p.To.Sym = Linksym(sym)
-	return sym
-}
-
 // TODO(mdempsky): Update to reference OpVar{Def,Kill,Live} instead
 // and move to plive.go.
 
@@ -113,6 +80,7 @@ func emitptrargsmap() {
 		return
 	}
 	sym := lookup(fmt.Sprintf("%s.args_stackmap", Curfn.Func.Nname.Sym.Name))
+	lsym := Linksym(sym)
 
 	nptr := int(Curfn.Type.ArgWidth() / int64(Widthptr))
 	bv := bvalloc(int32(nptr) * 2)
@@ -120,8 +88,8 @@ func emitptrargsmap() {
 	if Curfn.Type.Results().NumFields() > 0 {
 		nbitmap = 2
 	}
-	off := duint32(sym, 0, uint32(nbitmap))
-	off = duint32(sym, off, uint32(bv.n))
+	off := duint32LSym(lsym, 0, uint32(nbitmap))
+	off = duint32LSym(lsym, off, uint32(bv.n))
 	var xoffset int64
 	if Curfn.IsMethod() {
 		xoffset = 0
@@ -133,14 +101,14 @@ func emitptrargsmap() {
 		onebitwalktype1(Curfn.Type.Params(), &xoffset, bv)
 	}
 
-	off = dbvec(sym, off, bv)
+	off = dbvecLSym(lsym, off, bv)
 	if Curfn.Type.Results().NumFields() > 0 {
 		xoffset = 0
 		onebitwalktype1(Curfn.Type.Results(), &xoffset, bv)
-		off = dbvec(sym, off, bv)
+		off = dbvecLSym(lsym, off, bv)
 	}
 
-	ggloblsym(sym, int32(off), obj.RODATA|obj.LOCAL)
+	ggloblLSym(lsym, int32(off), obj.RODATA|obj.LOCAL)
 }
 
 // cmpstackvarlt reports whether the stack variable a sorts before b.

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

@@ -1051,32 +1051,18 @@ func livenessprintdebug(lv *Liveness) {
 	fmt.Printf("\n")
 }
 
-func finishgclocals(sym *types.Sym) {
-	ls := Linksym(sym)
-	ls.Name = fmt.Sprintf("gclocals·%x", md5.Sum(ls.P))
-	ls.Set(obj.AttrDuplicateOK, true)
-	sv := obj.SymVer{Name: ls.Name, Version: 0}
-	ls2, ok := Ctxt.Hash[sv]
-	if ok {
-		sym.Lsym = ls2
-	} else {
-		Ctxt.Hash[sv] = ls
-		ggloblsym(sym, int32(ls.Size), obj.RODATA)
-	}
-}
-
 // Dumps a slice of bitmaps to a symbol as a sequence of uint32 values. The
 // first word dumped is the total number of bitmaps. The second word is the
 // length of the bitmaps. All bitmaps are assumed to be of equal length. The
 // remaining bytes are the raw bitmaps.
-func livenessemit(lv *Liveness, argssym, livesym *types.Sym) {
+func livenessemit(lv *Liveness, argssym, livesym *obj.LSym) {
 	args := bvalloc(argswords(lv))
-	aoff := duint32(argssym, 0, uint32(len(lv.livevars))) // number of bitmaps
-	aoff = duint32(argssym, aoff, uint32(args.n))         // number of bits in each bitmap
+	aoff := duint32LSym(argssym, 0, uint32(len(lv.livevars))) // number of bitmaps
+	aoff = duint32LSym(argssym, aoff, uint32(args.n))         // number of bits in each bitmap
 
 	locals := bvalloc(localswords(lv))
-	loff := duint32(livesym, 0, uint32(len(lv.livevars))) // number of bitmaps
-	loff = duint32(livesym, loff, uint32(locals.n))       // number of bits in each bitmap
+	loff := duint32LSym(livesym, 0, uint32(len(lv.livevars))) // number of bitmaps
+	loff = duint32LSym(livesym, loff, uint32(locals.n))       // number of bits in each bitmap
 
 	for _, live := range lv.livevars {
 		args.Clear()
@@ -1084,19 +1070,24 @@ func livenessemit(lv *Liveness, argssym, livesym *types.Sym) {
 
 		onebitlivepointermap(lv, live, lv.vars, args, locals)
 
-		aoff = dbvec(argssym, aoff, args)
-		loff = dbvec(livesym, loff, locals)
+		aoff = dbvecLSym(argssym, aoff, args)
+		loff = dbvecLSym(livesym, loff, locals)
 	}
 
-	finishgclocals(livesym)
-	finishgclocals(argssym)
+	// Give these LSyms content-addressable names,
+	// so that they can be de-duplicated.
+	// This provides significant binary size savings.
+	// It is safe to rename these LSyms because
+	// they are tracked separately from ctxt.Hash.
+	argssym.Name = fmt.Sprintf("gclocals·%x", md5.Sum(argssym.P))
+	livesym.Name = fmt.Sprintf("gclocals·%x", md5.Sum(livesym.P))
 }
 
 // Entry pointer for liveness analysis. Solves for the liveness of
 // pointer variables in the function and emits a runtime data
 // structure read by the garbage collector.
 // Returns a map from GC safe points to their corresponding stack map index.
-func liveness(e *ssafn, f *ssa.Func, argssym, livesym *types.Sym) map[*ssa.Value]int {
+func liveness(e *ssafn, f *ssa.Func) map[*ssa.Value]int {
 	// Construct the global liveness state.
 	vars := getvariables(e.curfn)
 	lv := newliveness(e.curfn, f, vars, e.stkptrsize)
@@ -1111,6 +1102,8 @@ func liveness(e *ssafn, f *ssa.Func, argssym, livesym *types.Sym) map[*ssa.Value
 	}
 
 	// Emit the live pointer map data structures
-	livenessemit(lv, argssym, livesym)
+	if ls := e.curfn.Func.lsym; ls != nil {
+		livenessemit(lv, &ls.FuncInfo.GCArgs, &ls.FuncInfo.GCLocals)
+	}
 	return lv.stackMapIndex
 }

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

@@ -4326,9 +4326,7 @@ func genssa(f *ssa.Func, pp *Progs) {
 	e := f.Frontend().(*ssafn)
 
 	// Generate GC bitmaps.
-	gcargs := makefuncdatasym(pp, "gcargs·", obj.FUNCDATA_ArgsPointerMaps, e.curfn)
-	gclocals := makefuncdatasym(pp, "gclocals·", obj.FUNCDATA_LocalsPointerMaps, e.curfn)
-	s.stackMapIndex = liveness(e, f, gcargs, gclocals)
+	s.stackMapIndex = liveness(e, f)
 
 	// Remember where each block starts.
 	s.bstart = make([]*obj.Prog, f.NumBlocks())

src/cmd/internal/obj/link.go

@@ -330,6 +330,8 @@ type FuncInfo struct {
 	Autom    []*Auto
 	Pcln     Pcln
 	dwarfSym *LSym
+	GCArgs   LSym
+	GCLocals LSym
 }
 
 // Attribute is a set of symbol attributes.

src/cmd/internal/obj/plist.go

@@ -139,6 +139,15 @@ func (ctxt *Link) InitTextSym(s *LSym, flag int) {
 	dsym.Type = SDWARFINFO
 	dsym.Set(AttrDuplicateOK, s.DuplicateOK())
 	ctxt.Data = append(ctxt.Data, dsym)
+
+	// Set up the function's gcargs and gclocals.
+	// They will be filled in later if needed.
+	gcargs := &s.FuncInfo.GCArgs
+	gcargs.Set(AttrDuplicateOK, true)
+	gcargs.Type = SRODATA
+	gclocals := &s.FuncInfo.GCLocals
+	gclocals.Set(AttrDuplicateOK, true)
+	gclocals.Type = SRODATA
 }
 
 func (ctxt *Link) Globl(s *LSym, size int64, flag int) {

src/cmd/link/internal/ld/objfile.go

@@ -479,7 +479,7 @@ func (r *objReader) readRef() {
 	name := r.readSymName()
 	v := r.readInt()
 	if v != 0 && v != 1 {
-		log.Fatalf("invalid symbol version %d", v)
+		log.Fatalf("invalid symbol version for %q: %d", name, v)
 	}
 	if v == 1 {
 		v = r.localSymVersion