cmd/compile: add OSTRUCTKEY for keyed struct literals

Previously, we used OKEY nodes to represent keyed struct literal
elements. The field names were represented by an ONAME node, but this
is clumsy because it's the only remaining case where ONAME was used to
represent a bare identifier and not a variable.

This CL introduces a new OSTRUCTKEY node op for use in struct
literals. These ops instead store the field name in the node's own Sym
field. This is similar in spirit to golang.org/cl/20890.

Significant reduction in allocations for struct literal heavy code
like package unicode:

name       old time/op     new time/op     delta
Template       345ms ± 6%      341ms ± 6%     ~           (p=0.141 n=29+28)
Unicode        200ms ± 9%      184ms ± 7%   -7.77%        (p=0.000 n=29+30)
GoTypes        1.04s ± 3%      1.05s ± 3%     ~           (p=0.096 n=30+30)
Compiler       4.47s ± 9%      4.49s ± 6%     ~           (p=0.890 n=29+29)

name       old user-ns/op  new user-ns/op  delta
Template        523M ±13%       516M ±17%     ~           (p=0.400 n=29+30)
Unicode         334M ±27%       314M ±30%     ~           (p=0.093 n=30+30)
GoTypes        1.53G ±10%      1.52G ±10%     ~           (p=0.572 n=30+30)
Compiler       6.28G ± 7%      6.34G ±11%     ~           (p=0.300 n=30+30)

name       old alloc/op    new alloc/op    delta
Template      44.5MB ± 0%     44.4MB ± 0%   -0.35%        (p=0.000 n=27+30)
Unicode       39.2MB ± 0%     34.5MB ± 0%  -11.79%        (p=0.000 n=26+30)
GoTypes        125MB ± 0%      125MB ± 0%   -0.12%        (p=0.000 n=29+30)
Compiler       515MB ± 0%      515MB ± 0%   -0.10%        (p=0.000 n=29+30)

name       old allocs/op   new allocs/op   delta
Template        426k ± 0%       424k ± 0%   -0.39%        (p=0.000 n=29+30)
Unicode         374k ± 0%       323k ± 0%  -13.67%        (p=0.000 n=29+30)
GoTypes        1.21M ± 0%      1.21M ± 0%   -0.14%        (p=0.000 n=29+29)
Compiler       4.40M ± 0%      4.39M ± 0%   -0.13%        (p=0.000 n=29+30)

Passes toolstash/buildall.

Change-Id: Iba4ee765dd1748f67e52fcade1cd75c9f6e13fa9
Reviewed-on: https://go-review.googlesource.com/30974
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>

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

@@ -1154,8 +1154,8 @@ func (p *exporter) elemList(list Nodes) {
 		if p.trace {
 			p.tracef("\n")
 		}
-		p.fieldSym(n.Left.Sym, false)
+		p.fieldSym(n.Sym, false)
-		p.expr(n.Right)
+		p.expr(n.Left)
 	}
 }
 
@@ -1267,6 +1267,9 @@ func (p *exporter) expr(n *Node) {
 		p.op(OKEY)
 		p.exprsOrNil(n.Left, n.Right)
 
+	// case OSTRUCTKEY:
+	//	unreachable - handled in case OSTRUCTLIT by elemList
+
 	// case OCALLPART:
 	//	unimplemented - handled by default case
 

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

@@ -804,7 +804,8 @@ func (p *importer) elemList() []*Node {
 	c := p.int()
 	list := make([]*Node, c)
 	for i := range list {
-		list[i] = nod(OKEY, mkname(p.fieldSym()), p.expr())
+		s := p.fieldSym()
+		list[i] = nodSym(OSTRUCTKEY, p.expr(), s)
 	}
 	return list
 }
@@ -897,6 +898,9 @@ func (p *importer) node() *Node {
 		left, right := p.exprsOrNil()
 		return nod(OKEY, left, right)
 
+	// case OSTRUCTKEY:
+	//	unreachable - handled in case OSTRUCTLIT by elemList
+
 	// case OCALLPART:
 	//	unimplemented
 

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

@@ -633,12 +633,6 @@ func (e *EscState) esc(n *Node, parent *Node) {
 	if n == nil {
 		return
 	}
-	if n.Type == structkey {
-		// This is the left side of x:y in a struct literal.
-		// x is syntax, not an expression.
-		// See #14405.
-		return
-	}
 
 	lno := setlineno(n)
 
@@ -905,7 +899,7 @@ func (e *EscState) esc(n *Node, parent *Node) {
 		// Link values to struct.
 	case OSTRUCTLIT:
 		for _, n6 := range n.List.Slice() {
-			e.escassignNilWhy(n, n6.Right, "struct literal element")
+			e.escassignNilWhy(n, n6.Left, "struct literal element")
 		}
 
 	case OPTRLIT:

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

@@ -1272,6 +1272,9 @@ func (n *Node) exprfmt(s fmt.State, prec int) {
 		}
 		fmt.Fprint(s, ":")
 
+	case OSTRUCTKEY:
+		fmt.Fprintf(s, "%v:%v", n.Sym, n.Left)
+
 	case OCALLPART:
 		n.Left.exprfmt(s, nprec)
 		if n.Right == nil || n.Right.Sym == nil {

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

@@ -248,6 +248,10 @@ func ishairy(n *Node, budget *int32, reason *string) bool {
 	}
 
 	(*budget)--
+	// TODO(mdempsky): Hack to appease toolstash; remove.
+	if n.Op == OSTRUCTKEY {
+		(*budget)--
+	}
 
 	return *budget < 0 || ishairy(n.Left, budget, reason) || ishairy(n.Right, budget, reason) ||
 		ishairylist(n.List, budget, reason) || ishairylist(n.Rlist, budget, reason) ||

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

@@ -77,6 +77,7 @@ var opnames = []string{
 	OINDEX:           "INDEX",
 	OINDEXMAP:        "INDEXMAP",
 	OKEY:             "KEY",
+	OSTRUCTKEY:       "STRUCTKEY",
 	OLEN:             "LEN",
 	OMAKE:            "MAKE",
 	OMAKECHAN:        "MAKECHAN",

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

@@ -315,11 +315,6 @@ func instrumentnode(np **Node, init *Nodes, wr int, skip int) {
 		n.SetSliceBounds(low, high, max)
 		goto ret
 
-	case OKEY:
-		instrumentnode(&n.Left, init, 0, 0)
-		instrumentnode(&n.Right, init, 0, 0)
-		goto ret
-
 	case OADDR:
 		instrumentnode(&n.Left, init, 0, 1)
 		goto ret

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

@@ -622,6 +622,9 @@ func getdyn(n *Node, top bool) initGenType {
 	var mode initGenType
 	for _, n1 := range n.List.Slice() {
 		value := n1.Right
+		if n.Op == OSTRUCTLIT {
+			value = n1.Left
+		}
 		mode |= getdyn(value, false)
 		if mode == initDynamic|initConst {
 			break
@@ -635,17 +638,22 @@ func isStaticCompositeLiteral(n *Node) bool {
 	switch n.Op {
 	case OSLICELIT:
 		return false
-	case OARRAYLIT, OSTRUCTLIT:
+	case OARRAYLIT:
 		for _, r := range n.List.Slice() {
 			if r.Op != OKEY {
 				Fatalf("isStaticCompositeLiteral: rhs not OKEY: %v", r)
 			}
-			index := r.Left
+			if r.Left.Op != OLITERAL || !isStaticCompositeLiteral(r.Right) {
-			if n.Op == OARRAYLIT && index.Op != OLITERAL {
 				return false
 			}
-			value := r.Right
+		}
-			if !isStaticCompositeLiteral(value) {
+		return true
+	case OSTRUCTLIT:
+		for _, r := range n.List.Slice() {
+			if r.Op != OSTRUCTKEY {
+				Fatalf("isStaticCompositeLiteral: rhs not OSTRUCTKEY: %v", r)
+			}
+			if !isStaticCompositeLiteral(r.Left) {
 				return false
 			}
 		}
@@ -689,40 +697,44 @@ const (
 // fixedlit handles struct, array, and slice literals.
 // TODO: expand documentation.
 func fixedlit(ctxt initContext, kind initKind, n *Node, var_ *Node, init *Nodes) {
-	var indexnode func(*Node) *Node
+	var splitnode func(*Node) (a *Node, value *Node)
 	switch n.Op {
 	case OARRAYLIT, OSLICELIT:
-		indexnode = func(index *Node) *Node { return nod(OINDEX, var_, index) }
+		splitnode = func(r *Node) (*Node, *Node) {
+			if r.Op != OKEY {
+				Fatalf("fixedlit: rhs not OKEY: %v", r)
+			}
+			return nod(OINDEX, var_, r.Left), r.Right
+		}
 	case OSTRUCTLIT:
-		indexnode = func(index *Node) *Node { return nodSym(ODOT, var_, index.Sym) }
+		splitnode = func(r *Node) (*Node, *Node) {
+			if r.Op != OSTRUCTKEY {
+				Fatalf("fixedlit: rhs not OSTRUCTKEY: %v", r)
+			}
+			return nodSym(ODOT, var_, r.Sym), r.Left
+		}
 	default:
 		Fatalf("fixedlit bad op: %v", n.Op)
 	}
 
 	for _, r := range n.List.Slice() {
-		if r.Op != OKEY {
+		a, value := splitnode(r)
-			Fatalf("fixedlit: rhs not OKEY: %v", r)
-		}
-		index := r.Left
-		value := r.Right
 
 		switch value.Op {
 		case OSLICELIT:
 			if (kind == initKindStatic && ctxt == inNonInitFunction) || (kind == initKindDynamic && ctxt == inInitFunction) {
-				a := indexnode(index)
 				slicelit(ctxt, value, a, init)
 				continue
 			}
 
 		case OARRAYLIT, OSTRUCTLIT:
-			a := indexnode(index)
 			fixedlit(ctxt, kind, value, a, init)
 			continue
 		}
 
 		islit := isliteral(value)
 		if n.Op == OARRAYLIT {
-			islit = islit && isliteral(index)
+			islit = islit && isliteral(r.Left)
 		}
 		if (kind == initKindStatic && !islit) || (kind == initKindDynamic && islit) {
 			continue
@@ -730,7 +742,7 @@ func fixedlit(ctxt initContext, kind initKind, n *Node, var_ *Node, init *Nodes)
 
 		// build list of assignments: var[index] = expr
 		setlineno(value)
-		a := nod(OAS, indexnode(index), value)
+		a = nod(OAS, a, value)
 		a = typecheck(a, Etop)
 		switch kind {
 		case initKindStatic:
@@ -1235,10 +1247,10 @@ func initplan(n *Node) {
 
 	case OSTRUCTLIT:
 		for _, a := range n.List.Slice() {
-			if a.Op != OKEY || a.Left.Type != structkey {
+			if a.Op != OSTRUCTKEY {
 				Fatalf("initplan fixedlit")
 			}
-			addvalue(p, a.Left.Xoffset, a.Right)
+			addvalue(p, a.Xoffset, a.Left)
 		}
 
 	case OMAPLIT:
@@ -1294,13 +1306,21 @@ func iszero(n *Node) bool {
 			return u.Real.CmpFloat64(0) == 0 && u.Imag.CmpFloat64(0) == 0
 		}
 
-	case OARRAYLIT, OSTRUCTLIT:
+	case OARRAYLIT:
 		for _, n1 := range n.List.Slice() {
 			if !iszero(n1.Right) {
 				return false
 			}
 		}
 		return true
+
+	case OSTRUCTLIT:
+		for _, n1 := range n.List.Slice() {
+			if !iszero(n1.Left) {
+				return false
+			}
+		}
+		return true
 	}
 
 	return false

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

@@ -33,12 +33,11 @@ type Node struct {
 	Sym *Sym        // various
 	E   interface{} // Opt or Val, see methods below
 
-	// Various. Usually an offset into a struct. For example, ONAME nodes
+	// Various. Usually an offset into a struct. For example:
-	// that refer to local variables use it to identify their stack frame
+	// - ONAME nodes that refer to local variables use it to identify their stack frame position.
-	// position. ODOT, ODOTPTR, and OINDREG use it to indicate offset
+	// - ODOT, ODOTPTR, and OINDREG use it to indicate offset relative to their base address.
-	// relative to their base address. ONAME nodes on the left side of an
+	// - OSTRUCTKEY uses it to store the named field's offset.
-	// OKEY within an OSTRUCTLIT use it to store the named field's offset.
+	// - OXCASE and OXFALL use it to validate the use of fallthrough.
-	// OXCASE and OXFALL use it to validate the use of fallthrough.
 	// Possibly still more uses. If you find any, document them.
 	Xoffset int64
 
@@ -484,6 +483,9 @@ const (
 	OGETG   // runtime.getg() (read g pointer)
 
 	OEND
+
+	// TODO(mdempsky): Hack to appease toolstash; move up next to OKEY.
+	OSTRUCTKEY // Sym:Left (key:value in struct literal, after type checking)
 )
 
 // Nodes is a pointer to a slice of *Node.

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

@@ -2504,7 +2504,7 @@ func lookdot(n *Node, t *Type, dostrcmp int) *Field {
 
 func nokeys(l Nodes) bool {
 	for _, n := range l.Slice() {
-		if n.Op == OKEY {
+		if n.Op == OKEY || n.Op == OSTRUCTKEY {
 			return false
 		}
 	}
@@ -2737,11 +2737,7 @@ func (nl Nodes) retsigerr() string {
 }
 
 // type check composite
-func fielddup(n *Node, hash map[string]bool) {
+func fielddup(name string, hash map[string]bool) {
-	if n.Op != ONAME {
-		Fatalf("fielddup: not ONAME")
-	}
-	name := n.Sym.Name
 	if hash[name] {
 		yyerror("duplicate field name in struct literal: %s", name)
 		return
@@ -2839,11 +2835,6 @@ func pushtype(n *Node, t *Type) {
 	}
 }
 
-// Marker type so esc, fmt, and sinit can recognize the LHS of an OKEY node
-// in a struct literal.
-// TODO(mdempsky): Find a nicer solution.
-var structkey = typ(Txxx)
-
 // The result of typecheckcomplit MUST be assigned back to n, e.g.
 // 	n.Left = typecheckcomplit(n.Left)
 func typecheckcomplit(n *Node) *Node {
@@ -3029,10 +3020,8 @@ func typecheckcomplit(n *Node) *Node {
 				}
 				// No pushtype allowed here. Must name fields for that.
 				n1 = assignconv(n1, f.Type, "field value")
-				n1 = nod(OKEY, newname(f.Sym), n1)
+				n1 = nodSym(OSTRUCTKEY, n1, f.Sym)
-				n1.Left.Type = structkey
+				n1.Xoffset = f.Offset
-				n1.Left.Xoffset = f.Offset
-				n1.Left.Typecheck = 1
 				ls[i1] = n1
 				f = it.Next()
 			}
@@ -3047,7 +3036,38 @@ func typecheckcomplit(n *Node) *Node {
 			ls := n.List.Slice()
 			for i, l := range ls {
 				setlineno(l)
-				if l.Op != OKEY {
+
+				if l.Op == OKEY {
+					key := l.Left
+
+					l.Op = OSTRUCTKEY
+					l.Left = l.Right
+					l.Right = nil
+
+					// An OXDOT uses the Sym field to hold
+					// the field to the right of the dot,
+					// so s will be non-nil, but an OXDOT
+					// is never a valid struct literal key.
+					if key.Sym == nil || key.Op == OXDOT {
+						yyerror("invalid field name %v in struct initializer", key)
+						l.Left = typecheck(l.Left, Erv)
+						continue
+					}
+
+					// Sym might have resolved to name in other top-level
+					// package, because of import dot. Redirect to correct sym
+					// before we do the lookup.
+					s := key.Sym
+					if s.Pkg != localpkg && exportname(s.Name) {
+						s1 := lookup(s.Name)
+						if s1.Origpkg == s.Pkg {
+							s = s1
+						}
+					}
+					l.Sym = s
+				}
+
+				if l.Op != OSTRUCTKEY {
 					if bad == 0 {
 						yyerror("mixture of field:value and value initializers")
 					}
@@ -3056,46 +3076,17 @@ func typecheckcomplit(n *Node) *Node {
 					continue
 				}
 
-				s := l.Left.Sym
+				f := lookdot1(nil, l.Sym, t, t.Fields(), 0)
-
-				// An OXDOT uses the Sym field to hold
-				// the field to the right of the dot,
-				// so s will be non-nil, but an OXDOT
-				// is never a valid struct literal key.
-				if s == nil || l.Left.Op == OXDOT {
-					yyerror("invalid field name %v in struct initializer", l.Left)
-					l.Right = typecheck(l.Right, Erv)
-					continue
-				}
-
-				// Sym might have resolved to name in other top-level
-				// package, because of import dot. Redirect to correct sym
-				// before we do the lookup.
-				if s.Pkg != localpkg && exportname(s.Name) {
-					s1 := lookup(s.Name)
-					if s1.Origpkg == s.Pkg {
-						s = s1
-					}
-				}
-
-				f := lookdot1(nil, s, t, t.Fields(), 0)
 				if f == nil {
-					yyerror("unknown %v field '%v' in struct literal", t, s)
+					yyerror("unknown %v field '%v' in struct literal", t, l.Sym)
 					continue
 				}
-
+				fielddup(f.Sym.Name, hash)
-				l.Left = newname(s)
+				l.Xoffset = f.Offset
-				l.Left.Type = structkey
-				l.Left.Xoffset = f.Offset
-				l.Left.Typecheck = 1
-				s = f.Sym
-				fielddup(newname(s), hash)
-				r = l.Right
 
 				// No pushtype allowed here. Tried and rejected.
-				r = typecheck(r, Erv)
+				l.Left = typecheck(l.Left, Erv)
-
+				l.Left = assignconv(l.Left, f.Type, "field value")
-				l.Right = assignconv(r, f.Type, "field value")
 			}
 		}
 

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

@@ -486,13 +486,6 @@ func walkexpr(n *Node, init *Nodes) *Node {
 		init.AppendNodes(&n.Ninit)
 	}
 
-	// annoying case - not typechecked
-	if n.Op == OKEY {
-		n.Left = walkexpr(n.Left, init)
-		n.Right = walkexpr(n.Right, init)
-		return n
-	}
-
 	lno := setlineno(n)
 
 	if Debug['w'] > 1 {
@@ -4100,6 +4093,7 @@ func candiscard(n *Node) bool {
 		OGT,
 		OGE,
 		OKEY,
+		OSTRUCTKEY,
 		OLEN,
 		OMUL,
 		OLSH,