cmd/compile: replace Field.Nname.Pos with Field.Pos

For struct fields and methods, Field.Nname was only used to store
position information, which means we're allocating an entire ONAME
Node+Name+Param structure just for one field. We can optimize away
these ONAME allocations by instead adding a Field.Pos field.

Unfortunately, we can't get rid of Field.Nname, because it's needed
for function parameters, so Field grows a little bit and now has more
redundant information in those cases. However, that was already the
case (e.g., Field.Sym and Field.Nname.Sym), and it's still a net win
for allocations as demonstrated by the benchmarks below.

Additionally, by moving the ONAME allocation for function parameters
to funcargs, we can avoid allocating them for function parameters that
aren't used in corresponding function bodies (e.g., interface methods,
function-typed variables, and imported functions/methods without
inline bodies).

name       old time/op       new time/op       delta
Template         254ms ± 6%        251ms ± 6%  -1.04%  (p=0.000 n=487+488)
Unicode          128ms ± 7%        128ms ± 7%    ~     (p=0.294 n=482+467)
GoTypes          862ms ± 5%        860ms ± 4%    ~     (p=0.075 n=488+471)
Compiler         3.91s ± 4%        3.90s ± 4%  -0.39%  (p=0.000 n=468+473)

name       old user-time/op  new user-time/op  delta
Template         339ms ±14%        336ms ±14%  -1.02%  (p=0.001 n=498+494)
Unicode          176ms ±18%        176ms ±25%    ~     (p=0.940 n=491+499)
GoTypes          1.13s ± 8%        1.13s ± 9%    ~     (p=0.157 n=496+493)
Compiler         5.24s ± 6%        5.21s ± 6%  -0.57%  (p=0.000 n=485+489)

name       old alloc/op      new alloc/op      delta
Template        38.3MB ± 0%       37.3MB ± 0%  -2.58%  (p=0.000 n=499+497)
Unicode         29.1MB ± 0%       29.1MB ± 0%  -0.03%  (p=0.000 n=500+493)
GoTypes          116MB ± 0%        115MB ± 0%  -0.65%  (p=0.000 n=498+499)
Compiler         492MB ± 0%        487MB ± 0%  -1.00%  (p=0.000 n=497+498)

name       old allocs/op     new allocs/op     delta
Template          364k ± 0%         360k ± 0%  -1.15%  (p=0.000 n=499+499)
Unicode           336k ± 0%         336k ± 0%  -0.01%  (p=0.000 n=500+493)
GoTypes          1.16M ± 0%        1.16M ± 0%  -0.30%  (p=0.000 n=499+499)
Compiler         4.54M ± 0%        4.51M ± 0%  -0.58%  (p=0.000 n=494+495)

Passes toolstash-check -gcflags=-dwarf=false. Changes DWARF output
because position information is now tracked more precisely for
function parameters.

Change-Id: Ib8077d70d564cc448c5e4290baceab3a4396d712
Reviewed-on: https://go-review.googlesource.com/108217
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Robert Griesemer <gri@golang.org>

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

@@ -36,7 +36,7 @@ func expandiface(t *types.Type) {
 		}
 
 		if !m.Type.IsInterface() {
-			yyerrorl(asNode(m.Nname).Pos, "interface contains embedded non-interface %v", m.Type)
+			yyerrorl(m.Pos, "interface contains embedded non-interface %v", m.Type)
 			m.SetBroke(true)
 			t.SetBroke(true)
 			// Add to fields so that error messages
@@ -52,10 +52,10 @@ func expandiface(t *types.Type) {
 		// method set.
 		for _, t1 := range m.Type.Fields().Slice() {
 			f := types.NewField()
+			f.Pos = m.Pos // preserve embedding position
+			f.Sym = t1.Sym
 			f.Type = t1.Type
 			f.SetBroke(t1.Broke())
-			f.Sym = t1.Sym
-			f.Nname = m.Nname // preserve embedding position
 			fields = append(fields, f)
 		}
 	}
@@ -100,7 +100,7 @@ func widstruct(errtype *types.Type, t *types.Type, o int64, flag int) int64 {
 			o = Rnd(o, int64(f.Type.Align))
 		}
 		f.Offset = o
-		if asNode(f.Nname) != nil {
+		if n := asNode(f.Nname); n != nil {
 			// addrescapes has similar code to update these offsets.
 			// Usually addrescapes runs after widstruct,
 			// in which case we could drop this,
@@ -108,11 +108,11 @@ func widstruct(errtype *types.Type, t *types.Type, o int64, flag int) int64 {
 			// NOTE(rsc): This comment may be stale.
 			// It's possible the ordering has changed and this is
 			// now the common case. I'm not sure.
-			if asNode(f.Nname).Name.Param.Stackcopy != nil {
-				asNode(f.Nname).Name.Param.Stackcopy.Xoffset = o
-				asNode(f.Nname).Xoffset = 0
+			if n.Name.Param.Stackcopy != nil {
+				n.Name.Param.Stackcopy.Xoffset = o
+				n.Xoffset = 0
 			} else {
-				asNode(f.Nname).Xoffset = o
+				n.Xoffset = o
 			}
 		}
 

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

@@ -736,7 +736,7 @@ func (p *exporter) typ(t *types.Type) {
 				Fatalf("invalid symbol name: %s (%v)", m.Sym.Name, m.Sym)
 			}
 
-			p.pos(asNode(m.Nname).Pos)
+			p.pos(m.Pos)
 			p.fieldSym(m.Sym, false)
 
 			sig := m.Type
@@ -831,7 +831,7 @@ func (p *exporter) fieldList(t *types.Type) {
 }
 
 func (p *exporter) field(f *types.Field) {
-	p.pos(asNode(f.Nname).Pos)
+	p.pos(f.Pos)
 	p.fieldName(f)
 	p.typ(f.Type)
 	p.string(f.Note)
@@ -856,7 +856,7 @@ func (p *exporter) methodList(t *types.Type) {
 		if p.trace {
 			p.tracef("\n")
 		}
-		p.pos(asNode(m.Nname).Pos)
+		p.pos(m.Pos)
 		p.typ(m.Type)
 	}
 	if p.trace && len(embeddeds) > 0 {
@@ -879,11 +879,7 @@ func (p *exporter) methodList(t *types.Type) {
 }
 
 func (p *exporter) method(m *types.Field) {
-	if m.Nname != nil {
-		p.pos(asNode(m.Nname).Pos)
-	} else {
-		p.pos(src.NoXPos)
-	}
+	p.pos(m.Pos)
 	p.methodName(m.Sym)
 	p.paramList(m.Type.Params(), false)
 	p.paramList(m.Type.Results(), false)
@@ -1001,29 +997,19 @@ func (p *exporter) param(q *types.Field, n int, numbered bool) {
 }
 
 func parName(f *types.Field, numbered bool) string {
-	s := f.Sym
+	s := origSym(f.Sym)
 	if s == nil {
 		return ""
 	}
 
-	// Take the name from the original, lest we substituted it with ~r%d or ~b%d.
-	// ~r%d is a (formerly) unnamed result.
-	if asNode(f.Nname) != nil {
-		if asNode(f.Nname).Orig == nil {
-			return "" // s = nil
-		}
-		s = asNode(f.Nname).Orig.Sym
-		if s != nil && s.Name[0] == '~' {
-			if s.Name[1] == 'r' { // originally an unnamed result
-				return "" // s = nil
-			} else if s.Name[1] == 'b' { // originally the blank identifier _
-				return "_" // belongs to localpkg
-			}
-		}
-	}
-
-	if s == nil {
-		return ""
+	// The "s != f.Sym" check here is unnecessary and causes blank
+	// input/receiver parameters to receive vargen numbers
+	// below. However, this is consistent with the logic it
+	// replaces, so we keep it for now to appease toolstash-check.
+	//
+	// TODO(mdempsky): Simplify to just "if s.Name == "_"".
+	if s != f.Sym && s.Name == "_" {
+		return "_"
 	}
 
 	// print symbol with Vargen number or not as desired
@@ -1036,8 +1022,8 @@ func parName(f *types.Field, numbered bool) string {
 	// from other names in their context after inlining (i.e., the parameter numbering
 	// is a form of parameter rewriting). See issue 4326 for an example and test case.
 	if numbered {
-		if !strings.Contains(name, "·") && asNode(f.Nname) != nil && asNode(f.Nname).Name != nil && asNode(f.Nname).Name.Vargen > 0 {
-			name = fmt.Sprintf("%s·%d", name, asNode(f.Nname).Name.Vargen) // append Vargen
+		if n := asNode(f.Nname); !strings.Contains(name, "·") && n != nil && n.Name.Vargen > 0 {
+			name = fmt.Sprintf("%s·%d", name, n.Name.Vargen) // append Vargen
 		}
 	} else {
 		if i := strings.Index(name, "·"); i > 0 {

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

@@ -642,8 +642,8 @@ func (p *importer) field() *types.Field {
 		f.Embedded = 1
 	}
 
+	f.Pos = pos
 	f.Sym = sym
-	f.Nname = asTypesNode(newnamel(pos, sym))
 	f.Type = typ
 	f.Note = note
 
@@ -653,8 +653,7 @@ func (p *importer) field() *types.Field {
 func (p *importer) methodList() (methods []*types.Field) {
 	for n := p.int(); n > 0; n-- {
 		f := types.NewField()
-		f.Nname = asTypesNode(newname(nblank.Sym))
-		asNode(f.Nname).Pos = p.pos()
+		f.Pos = p.pos()
 		f.Type = p.typ()
 		methods = append(methods, f)
 	}
@@ -673,8 +672,8 @@ func (p *importer) method() *types.Field {
 	result := p.paramList()
 
 	f := types.NewField()
+	f.Pos = pos
 	f.Sym = sym
-	f.Nname = asTypesNode(newnamel(pos, sym))
 	f.Type = functypefield(fakeRecvField(), params, result)
 	return f
 }
@@ -743,6 +742,8 @@ func (p *importer) paramList() []*types.Field {
 
 func (p *importer) param(named bool) *types.Field {
 	f := types.NewField()
+	// TODO(mdempsky): Need param position.
+	f.Pos = lineno
 	f.Type = p.typ()
 	if f.Type.Etype == TDDDFIELD {
 		// TDDDFIELD indicates wrapped ... slice type
@@ -762,8 +763,6 @@ func (p *importer) param(named bool) *types.Field {
 			pkg = p.pkg()
 		}
 		f.Sym = pkg.Lookup(name)
-		// TODO(mdempsky): Need param position.
-		f.Nname = asTypesNode(newname(f.Sym))
 	}
 
 	// TODO(gri) This is compiler-specific (escape info).

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

@@ -908,10 +908,10 @@ func (n *Node) stmtfmt(s fmt.State, mode fmtMode) {
 		mode.Fprintf(s, "var %v %v", n.Left.Sym, n.Left.Type)
 
 	case ODCLFIELD:
-		if n.Left != nil {
-			mode.Fprintf(s, "%v %v", n.Left, n.Right)
+		if n.Sym != nil {
+			mode.Fprintf(s, "%v %v", n.Sym, n.Left)
 		} else {
-			mode.Fprintf(s, "%v", n.Right)
+			mode.Fprintf(s, "%v", n.Left)
 		}
 
 	// Don't export "v = <N>" initializing statements, hope they're always
@@ -1683,21 +1683,9 @@ func fldconv(f *types.Field, flag FmtFlag, mode fmtMode, depth int) string {
 	if flag&FmtShort == 0 {
 		s := f.Sym
 
-		// Take the name from the original, lest we substituted it with ~r%d or ~b%d.
-		// ~r%d is a (formerly) unnamed result.
-		if mode == FErr && asNode(f.Nname) != nil {
-			if asNode(f.Nname).Orig != nil {
-				s = asNode(f.Nname).Orig.Sym
-				if s != nil && s.Name[0] == '~' {
-					if s.Name[1] == 'r' { // originally an unnamed result
-						s = nil
-					} else if s.Name[1] == 'b' { // originally the blank identifier _
-						s = lookup("_")
-					}
-				}
-			} else {
-				s = nil
-			}
+		// Take the name from the original.
+		if mode == FErr {
+			s = origSym(s)
 		}
 
 		if s != nil && f.Embedded == 0 {

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

@@ -753,10 +753,10 @@ func mkinlcall(n *Node, fn *Node) *Node {
 }
 
 func tinlvar(t *types.Field, inlvars map[*Node]*Node) *Node {
-	if asNode(t.Nname) != nil && !asNode(t.Nname).isBlank() {
-		inlvar := inlvars[asNode(t.Nname)]
+	if n := asNode(t.Nname); n != nil && !n.isBlank() {
+		inlvar := inlvars[n]
 		if inlvar == nil {
-			Fatalf("missing inlvar for %v\n", asNode(t.Nname))
+			Fatalf("missing inlvar for %v\n", n)
 		}
 		return inlvar
 	}
@@ -884,12 +884,11 @@ func mkinlcall1(n, fn *Node) *Node {
 	var retvars []*Node
 	for i, t := range fn.Type.Results().Fields().Slice() {
 		var m *Node
-		var mpos src.XPos
-		if t != nil && asNode(t.Nname) != nil && !asNode(t.Nname).isBlank() {
-			mpos = asNode(t.Nname).Pos
-			m = inlvar(asNode(t.Nname))
+		mpos := t.Pos
+		if n := asNode(t.Nname); n != nil && !n.isBlank() {
+			m = inlvar(n)
 			m = typecheck(m, Erv)
-			inlvars[asNode(t.Nname)] = m
+			inlvars[n] = m
 		} else {
 			// anonymous return values, synthesize names for use in assignment that replaces return
 			m = retvar(t, i)

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

@@ -502,13 +502,13 @@ func (p *noder) params(params []*syntax.Field, dddOk bool) []*Node {
 }
 
 func (p *noder) param(param *syntax.Field, dddOk, final bool) *Node {
-	var name *Node
+	var name *types.Sym
 	if param.Name != nil {
-		name = p.newname(param.Name)
+		name = p.name(param.Name)
 	}
 
 	typ := p.typeExpr(param.Type)
-	n := p.nod(param, ODCLFIELD, name, typ)
+	n := p.nodSym(param, ODCLFIELD, typ, name)
 
 	// rewrite ...T parameter
 	if typ.Op == ODDD {
@@ -771,7 +771,7 @@ func (p *noder) structType(expr *syntax.StructType) *Node {
 		if field.Name == nil {
 			n = p.embedded(field.Type)
 		} else {
-			n = p.nod(field, ODCLFIELD, p.newname(field.Name), p.typeExpr(field.Type))
+			n = p.nodSym(field, ODCLFIELD, p.typeExpr(field.Type), p.name(field.Name))
 		}
 		if i < len(expr.TagList) && expr.TagList[i] != nil {
 			n.SetVal(p.basicLit(expr.TagList[i]))
@@ -791,12 +791,12 @@ func (p *noder) interfaceType(expr *syntax.InterfaceType) *Node {
 		p.lineno(method)
 		var n *Node
 		if method.Name == nil {
-			n = p.nod(method, ODCLFIELD, nil, oldname(p.packname(method.Type)))
+			n = p.nodSym(method, ODCLFIELD, oldname(p.packname(method.Type)), nil)
 		} else {
-			mname := p.newname(method.Name)
+			mname := p.name(method.Name)
 			sig := p.typeExpr(method.Type)
 			sig.Left = fakeRecv()
-			n = p.nod(method, ODCLFIELD, mname, sig)
+			n = p.nodSym(method, ODCLFIELD, sig, mname)
 			ifacedcl(n)
 		}
 		l = append(l, n)
@@ -840,11 +840,11 @@ func (p *noder) embedded(typ syntax.Expr) *Node {
 	}
 
 	sym := p.packname(typ)
-	n := nod(ODCLFIELD, newname(lookup(sym.Name)), oldname(sym))
+	n := p.nodSym(typ, ODCLFIELD, oldname(sym), lookup(sym.Name))
 	n.SetEmbedded(true)
 
 	if isStar {
-		n.Right = p.nod(op, OIND, n.Right, nil)
+		n.Left = p.nod(op, OIND, n.Left, nil)
 	}
 	return n
 }
@@ -1354,6 +1354,10 @@ func (p *noder) nod(orig syntax.Node, op Op, left, right *Node) *Node {
 	return p.setlineno(orig, nod(op, left, right))
 }
 
+func (p *noder) nodSym(orig syntax.Node, op Op, left *Node, sym *types.Sym) *Node {
+	return p.setlineno(orig, nodSym(op, left, sym))
+}
+
 func (p *noder) setlineno(src_ syntax.Node, dst *Node) *Node {
 	pos := src_.Pos()
 	if !pos.IsKnown() {

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

@@ -321,22 +321,19 @@ func hiter(t *types.Type) *types.Type {
 func methodfunc(f *types.Type, receiver *types.Type) *types.Type {
 	var in []*Node
 	if receiver != nil {
-		d := nod(ODCLFIELD, nil, nil)
-		d.Type = receiver
+		d := anonfield(receiver)
 		in = append(in, d)
 	}
 
 	for _, t := range f.Params().Fields().Slice() {
-		d := nod(ODCLFIELD, nil, nil)
-		d.Type = t.Type
+		d := anonfield(t.Type)
 		d.SetIsddd(t.Isddd())
 		in = append(in, d)
 	}
 
 	var out []*Node
 	for _, t := range f.Results().Fields().Slice() {
-		d := nod(ODCLFIELD, nil, nil)
-		d.Type = t.Type
+		d := anonfield(t.Type)
 		out = append(out, d)
 	}
 

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

@@ -1581,20 +1581,14 @@ func structargs(tl *types.Type, mustname bool) []*Node {
 	var args []*Node
 	gen := 0
 	for _, t := range tl.Fields().Slice() {
-		var n *Node
-		if mustname && (t.Sym == nil || t.Sym.Name == "_") {
+		s := t.Sym
+		if mustname && (s == nil || s.Name == "_") {
 			// invent a name so that we can refer to it in the trampoline
-			buf := fmt.Sprintf(".anon%d", gen)
+			s = lookupN(".anon", gen)
 			gen++
-			n = newname(lookup(buf))
-		} else if t.Sym != nil {
-			n = newname(t.Sym)
 		}
-		a := nod(ODCLFIELD, n, typenod(t.Type))
+		a := symfield(s, t.Type)
 		a.SetIsddd(t.Isddd())
-		if n != nil {
-			n.SetIsddd(t.Isddd())
-		}
 		args = append(args, a)
 	}
 

src/cmd/compile/internal/types/type.go

@@ -322,16 +322,18 @@ type Field struct {
 	Embedded uint8 // embedded field
 	Funarg   Funarg
 
-	Sym   *Sym
-	Nname *Node
+	Pos  src.XPos
+	Sym  *Sym
+	Type *Type  // field type
+	Note string // literal string annotation
 
-	Type *Type // field type
+	// For fields that represent function parameters, Nname points
+	// to the associated ONAME Node.
+	Nname *Node
 
 	// Offset in bytes of this field or method within its enclosing struct
 	// or interface Type.
 	Offset int64
-
-	Note string // literal string annotation
 }
 
 const (