[dev.regabi] cmd/compile: use ir.Find for "search" traversals

This CL converts all the generic searching traversal to use ir.Find
instead of relying on direct access to Left, Right, and so on.

Passes buildall w/ toolstash -cmp.

Change-Id: I4d951aef630c00bf333f24be79565cc564694d04
Reviewed-on: https://go-review.googlesource.com/c/go/+/275372
Trust: Russ Cox <rsc@golang.org>
Run-TryBot: Russ Cox <rsc@golang.org>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>

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

@@ -782,37 +782,14 @@ func geneq(t *types.Type) *obj.LSym {
 	return closure
 }
 
-func hasCall(n ir.Node) bool {
+func hasCall(fn *ir.Func) bool {
-	if n.Op() == ir.OCALL || n.Op() == ir.OCALLFUNC {
+	found := ir.Find(fn, func(n ir.Node) interface{} {
-		return true
+		if op := n.Op(); op == ir.OCALL || op == ir.OCALLFUNC {
+			return n
 		}
-	if n.Left() != nil && hasCall(n.Left()) {
+		return nil
-		return true
+	})
-	}
+	return found != nil
-	if n.Right() != nil && hasCall(n.Right()) {
-		return true
-	}
-	for _, x := range n.Init().Slice() {
-		if hasCall(x) {
-			return true
-		}
-	}
-	for _, x := range n.Body().Slice() {
-		if hasCall(x) {
-			return true
-		}
-	}
-	for _, x := range n.List().Slice() {
-		if hasCall(x) {
-			return true
-		}
-	}
-	for _, x := range n.Rlist().Slice() {
-		if hasCall(x) {
-			return true
-		}
-	}
-	return false
 }
 
 // eqfield returns the node

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

@@ -553,7 +553,7 @@ func evalConst(n ir.Node) ir.Node {
 				return origIntConst(n, int64(len(ir.StringVal(nl))))
 			}
 		case types.TARRAY:
-			if !hascallchan(nl) {
+			if !hasCallOrChan(nl) {
 				return origIntConst(n, nl.Type().NumElem())
 			}
 		}
@@ -779,10 +779,9 @@ func isGoConst(n ir.Node) bool {
 	return n.Op() == ir.OLITERAL
 }
 
-func hascallchan(n ir.Node) bool {
+// hasCallOrChan reports whether n contains any calls or channel operations.
-	if n == nil {
+func hasCallOrChan(n ir.Node) bool {
-		return false
+	found := ir.Find(n, func(n ir.Node) interface{} {
-	}
 		switch n.Op() {
 		case ir.OAPPEND,
 			ir.OCALL,
@@ -804,24 +803,11 @@ func hascallchan(n ir.Node) bool {
 			ir.OREAL,
 			ir.ORECOVER,
 			ir.ORECV:
-		return true
+			return n
 		}
-
+		return nil
-	if hascallchan(n.Left()) || hascallchan(n.Right()) {
+	})
-		return true
+	return found != nil
-	}
-	for _, n1 := range n.List().Slice() {
-		if hascallchan(n1) {
-			return true
-		}
-	}
-	for _, n2 := range n.Rlist().Slice() {
-		if hascallchan(n2) {
-			return true
-		}
-	}
-
-	return false
 }
 
 // A constSet represents a set of Go constant expressions.

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

@@ -33,6 +33,7 @@ import (
 	"cmd/compile/internal/types"
 	"cmd/internal/obj"
 	"cmd/internal/src"
+	"errors"
 	"fmt"
 	"go/constant"
 	"strings"
@@ -206,14 +207,10 @@ func caninl(fn *ir.Func) {
 		extraCallCost: cc,
 		usedLocals:    make(map[ir.Node]bool),
 	}
-	if visitor.visitList(fn.Body()) {
+	if visitor.tooHairy(fn) {
 		reason = visitor.reason
 		return
 	}
-	if visitor.budget < 0 {
-		reason = fmt.Sprintf("function too complex: cost %d exceeds budget %d", inlineMaxBudget-visitor.budget, inlineMaxBudget)
-		return
-	}
 
 	n.Func().Inl = &ir.Inline{
 		Cost: inlineMaxBudget - visitor.budget,
@@ -296,21 +293,29 @@ type hairyVisitor struct {
 	reason        string
 	extraCallCost int32
 	usedLocals    map[ir.Node]bool
+	do            func(ir.Node) error
 }
 
-// Look for anything we want to punt on.
+var errBudget = errors.New("too expensive")
-func (v *hairyVisitor) visitList(ll ir.Nodes) bool {
+
-	for _, n := range ll.Slice() {
+func (v *hairyVisitor) tooHairy(fn *ir.Func) bool {
-		if v.visit(n) {
+	v.do = v.doNode // cache closure
+
+	err := ir.DoChildren(fn, v.do)
+	if err != nil {
+		v.reason = err.Error()
 		return true
 	}
+	if v.budget < 0 {
+		v.reason = fmt.Sprintf("function too complex: cost %d exceeds budget %d", inlineMaxBudget-v.budget, inlineMaxBudget)
+		return true
 	}
 	return false
 }
 
-func (v *hairyVisitor) visit(n ir.Node) bool {
+func (v *hairyVisitor) doNode(n ir.Node) error {
 	if n == nil {
-		return false
+		return nil
 	}
 
 	switch n.Op() {
@@ -323,8 +328,7 @@ func (v *hairyVisitor) visit(n ir.Node) bool {
 		if n.Left().Op() == ir.ONAME && n.Left().Class() == ir.PFUNC && isRuntimePkg(n.Left().Sym().Pkg) {
 			fn := n.Left().Sym().Name
 			if fn == "getcallerpc" || fn == "getcallersp" {
-				v.reason = "call to " + fn
+				return errors.New("call to " + fn)
-				return true
 			}
 			if fn == "throw" {
 				v.budget -= inlineExtraThrowCost
@@ -380,8 +384,7 @@ func (v *hairyVisitor) visit(n ir.Node) bool {
 	case ir.ORECOVER:
 		// recover matches the argument frame pointer to find
 		// the right panic value, so it needs an argument frame.
-		v.reason = "call to recover"
+		return errors.New("call to recover")
-		return true
 
 	case ir.OCLOSURE,
 		ir.ORANGE,
@@ -390,21 +393,19 @@ func (v *hairyVisitor) visit(n ir.Node) bool {
 		ir.ODEFER,
 		ir.ODCLTYPE, // can't print yet
 		ir.ORETJMP:
-		v.reason = "unhandled op " + n.Op().String()
+		return errors.New("unhandled op " + n.Op().String())
-		return true
 
 	case ir.OAPPEND:
 		v.budget -= inlineExtraAppendCost
 
 	case ir.ODCLCONST, ir.OFALL:
 		// These nodes don't produce code; omit from inlining budget.
-		return false
+		return nil
 
 	case ir.OFOR, ir.OFORUNTIL, ir.OSWITCH:
 		// ORANGE, OSELECT in "unhandled" above
 		if n.Sym() != nil {
-			v.reason = "labeled control"
+			return errors.New("labeled control")
-			return true
 		}
 
 	case ir.OBREAK, ir.OCONTINUE:
@@ -416,8 +417,17 @@ func (v *hairyVisitor) visit(n ir.Node) bool {
 	case ir.OIF:
 		if ir.IsConst(n.Left(), constant.Bool) {
 			// This if and the condition cost nothing.
-			return v.visitList(n.Init()) || v.visitList(n.Body()) ||
+			// TODO(rsc): It seems strange that we visit the dead branch.
-				v.visitList(n.Rlist())
+			if err := ir.DoList(n.Init(), v.do); err != nil {
+				return err
+			}
+			if err := ir.DoList(n.Body(), v.do); err != nil {
+				return err
+			}
+			if err := ir.DoList(n.Rlist(), v.do); err != nil {
+				return err
+			}
+			return nil
 		}
 
 	case ir.ONAME:
@@ -439,34 +449,22 @@ func (v *hairyVisitor) visit(n ir.Node) bool {
 
 	// When debugging, don't stop early, to get full cost of inlining this function
 	if v.budget < 0 && base.Flag.LowerM < 2 && !logopt.Enabled() {
-		return true
+		return errBudget
 	}
 
-	return v.visit(n.Left()) || v.visit(n.Right()) ||
+	return ir.DoChildren(n, v.do)
-		v.visitList(n.List()) || v.visitList(n.Rlist()) ||
-		v.visitList(n.Init()) || v.visitList(n.Body())
 }
 
-func countNodes(n ir.Node) int {
+func isBigFunc(fn *ir.Func) bool {
-	if n == nil {
+	budget := inlineBigFunctionNodes
-		return 0
+	over := ir.Find(fn, func(n ir.Node) interface{} {
+		budget--
+		if budget <= 0 {
+			return n
 		}
-	cnt := 1
+		return nil
-	cnt += countNodes(n.Left())
+	})
-	cnt += countNodes(n.Right())
+	return over != nil
-	for _, n1 := range n.Init().Slice() {
-		cnt += countNodes(n1)
-	}
-	for _, n1 := range n.Body().Slice() {
-		cnt += countNodes(n1)
-	}
-	for _, n1 := range n.List().Slice() {
-		cnt += countNodes(n1)
-	}
-	for _, n1 := range n.Rlist().Slice() {
-		cnt += countNodes(n1)
-	}
-	return cnt
 }
 
 // Inlcalls/nodelist/node walks fn's statements and expressions and substitutes any
@@ -475,7 +473,7 @@ func inlcalls(fn *ir.Func) {
 	savefn := Curfn
 	Curfn = fn
 	maxCost := int32(inlineMaxBudget)
-	if countNodes(fn) >= inlineBigFunctionNodes {
+	if isBigFunc(fn) {
 		maxCost = inlineBigFunctionMaxCost
 	}
 	// Map to keep track of functions that have been inlined at a particular
@@ -742,82 +740,45 @@ FindRHS:
 		base.Fatalf("RHS is nil: %v", defn)
 	}
 
-	unsafe, _ := reassigned(n.(*ir.Name))
+	if reassigned(n.(*ir.Name)) {
-	if unsafe {
 		return nil
 	}
 
 	return rhs
 }
 
+var errFound = errors.New("found")
+
 // reassigned takes an ONAME node, walks the function in which it is defined, and returns a boolean
 // indicating whether the name has any assignments other than its declaration.
 // The second return value is the first such assignment encountered in the walk, if any. It is mostly
 // useful for -m output documenting the reason for inhibited optimizations.
 // NB: global variables are always considered to be re-assigned.
 // TODO: handle initial declaration not including an assignment and followed by a single assignment?
-func reassigned(n *ir.Name) (bool, ir.Node) {
+func reassigned(name *ir.Name) bool {
-	if n.Op() != ir.ONAME {
+	if name.Op() != ir.ONAME {
-		base.Fatalf("reassigned %v", n)
+		base.Fatalf("reassigned %v", name)
 	}
 	// no way to reliably check for no-reassignment of globals, assume it can be
-	if n.Curfn == nil {
+	if name.Curfn == nil {
-		return true, nil
+		return true
-	}
-	f := n.Curfn
-	v := reassignVisitor{name: n}
-	a := v.visitList(f.Body())
-	return a != nil, a
-}
-
-type reassignVisitor struct {
-	name ir.Node
-}
-
-func (v *reassignVisitor) visit(n ir.Node) ir.Node {
-	if n == nil {
-		return nil
 	}
+	a := ir.Find(name.Curfn, func(n ir.Node) interface{} {
 		switch n.Op() {
 		case ir.OAS:
-		if n.Left() == v.name && n != v.name.Name().Defn {
+			if n.Left() == name && n != name.Defn {
 				return n
 			}
 		case ir.OAS2, ir.OAS2FUNC, ir.OAS2MAPR, ir.OAS2DOTTYPE:
 			for _, p := range n.List().Slice() {
-			if p == v.name && n != v.name.Name().Defn {
+				if p == name && n != name.Defn {
 					return n
 				}
 			}
 		}
-	if a := v.visit(n.Left()); a != nil {
-		return a
-	}
-	if a := v.visit(n.Right()); a != nil {
-		return a
-	}
-	if a := v.visitList(n.List()); a != nil {
-		return a
-	}
-	if a := v.visitList(n.Rlist()); a != nil {
-		return a
-	}
-	if a := v.visitList(n.Init()); a != nil {
-		return a
-	}
-	if a := v.visitList(n.Body()); a != nil {
-		return a
-	}
-	return nil
-}
-
-func (v *reassignVisitor) visitList(l ir.Nodes) ir.Node {
-	for _, n := range l.Slice() {
-		if a := v.visit(n); a != nil {
-			return a
-		}
-	}
 		return nil
+	})
+	return a != nil
 }
 
 func inlParam(t *types.Field, as ir.Node, inlvars map[*ir.Name]ir.Node) ir.Node {
@@ -1140,6 +1101,7 @@ func mkinlcall(n ir.Node, fn *ir.Func, maxCost int32, inlMap map[*ir.Func]bool)
 		bases:        make(map[*src.PosBase]*src.PosBase),
 		newInlIndex:  newIndex,
 	}
+	subst.edit = subst.node
 
 	body := subst.list(ir.AsNodes(fn.Inl.Body))
 
@@ -1248,6 +1210,8 @@ type inlsubst struct {
 	// newInlIndex is the index of the inlined call frame to
 	// insert for inlined nodes.
 	newInlIndex int
+
+	edit func(ir.Node) ir.Node // cached copy of subst.node method value closure
 }
 
 // list inlines a list of nodes.
@@ -1334,21 +1298,13 @@ func (subst *inlsubst) node(n ir.Node) ir.Node {
 		return m
 	}
 
-	m := ir.Copy(n)
-	m.SetPos(subst.updatedPos(m.Pos()))
-	m.PtrInit().Set(nil)
-
 	if n.Op() == ir.OCLOSURE {
 		base.Fatalf("cannot inline function containing closure: %+v", n)
 	}
 
-	m.SetLeft(subst.node(n.Left()))
+	m := ir.Copy(n)
-	m.SetRight(subst.node(n.Right()))
+	m.SetPos(subst.updatedPos(m.Pos()))
-	m.PtrList().Set(subst.list(n.List()))
+	ir.EditChildren(m, subst.edit)
-	m.PtrRlist().Set(subst.list(n.Rlist()))
-	m.PtrInit().Set(append(m.Init().Slice(), subst.list(n.Init())...))
-	m.PtrBody().Set(subst.list(n.Body()))
-
 	return m
 }
 

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

@@ -1062,6 +1062,10 @@ func (o *Order) exprListInPlace(l ir.Nodes) {
 // prealloc[x] records the allocation to use for x.
 var prealloc = map[ir.Node]ir.Node{}
 
+func (o *Order) exprNoLHS(n ir.Node) ir.Node {
+	return o.expr(n, nil)
+}
+
 // expr orders a single expression, appending side
 // effects to o.out as needed.
 // If this is part of an assignment lhs = *np, lhs is given.
@@ -1079,10 +1083,7 @@ func (o *Order) expr(n, lhs ir.Node) ir.Node {
 
 	switch n.Op() {
 	default:
-		n.SetLeft(o.expr(n.Left(), nil))
+		ir.EditChildren(n, o.exprNoLHS)
-		n.SetRight(o.expr(n.Right(), nil))
-		o.exprList(n.List())
-		o.exprList(n.Rlist())
 
 	// Addition of strings turns into a function call.
 	// Allocate a temporary to hold the strings.

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

@@ -60,7 +60,8 @@ func (s *InitSchedule) tryStaticInit(n ir.Node) bool {
 	if n.Op() != ir.OAS {
 		return false
 	}
-	if ir.IsBlank(n.Left()) && candiscard(n.Right()) {
+	if ir.IsBlank(n.Left()) && !hasSideEffects(n.Right()) {
+		// Discard.
 		return true
 	}
 	lno := setlineno(n)
@@ -548,7 +549,8 @@ func fixedlit(ctxt initContext, kind initKind, n ir.Node, var_ ir.Node, init *ir
 
 	for _, r := range n.List().Slice() {
 		a, value := splitnode(r)
-		if a == ir.BlankNode && candiscard(value) {
+		if a == ir.BlankNode && !hasSideEffects(value) {
+			// Discard.
 			continue
 		}
 

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

@@ -3669,51 +3669,52 @@ func checkmake(t *types.Type, arg string, np *ir.Node) bool {
 	return true
 }
 
-func markbreak(labels *map[*types.Sym]ir.Node, n ir.Node, implicit ir.Node) {
+// markBreak marks control statements containing break statements with SetHasBreak(true).
-	if n == nil {
+func markBreak(fn *ir.Func) {
-		return
+	var labels map[*types.Sym]ir.Node
-	}
+	var implicit ir.Node
 
+	var mark func(ir.Node) error
+	mark = func(n ir.Node) error {
 		switch n.Op() {
+		default:
+			ir.DoChildren(n, mark)
+
 		case ir.OBREAK:
 			if n.Sym() == nil {
 				if implicit != nil {
 					implicit.SetHasBreak(true)
 				}
 			} else {
-			if lab := (*labels)[n.Sym()]; lab != nil {
+				if lab := labels[n.Sym()]; lab != nil {
 					lab.SetHasBreak(true)
 				}
 			}
+
 		case ir.OFOR, ir.OFORUNTIL, ir.OSWITCH, ir.OTYPESW, ir.OSELECT, ir.ORANGE:
+			old := implicit
 			implicit = n
-		if sym := n.Sym(); sym != nil {
+			sym := n.Sym()
-			if *labels == nil {
+			if sym != nil {
+				if labels == nil {
 					// Map creation delayed until we need it - most functions don't.
-				*labels = make(map[*types.Sym]ir.Node)
+					labels = make(map[*types.Sym]ir.Node)
 				}
-			(*labels)[sym] = n
+				labels[sym] = n
-			defer delete(*labels, sym)
 			}
-		fallthrough
+			ir.DoChildren(n, mark)
-	default:
+			if sym != nil {
-		markbreak(labels, n.Left(), implicit)
+				delete(labels, sym)
-		markbreak(labels, n.Right(), implicit)
-		markbreaklist(labels, n.Init(), implicit)
-		markbreaklist(labels, n.Body(), implicit)
-		markbreaklist(labels, n.List(), implicit)
-		markbreaklist(labels, n.Rlist(), implicit)
 			}
+			implicit = old
+		}
+		return nil
 	}
 
-func markbreaklist(labels *map[*types.Sym]ir.Node, l ir.Nodes, implicit ir.Node) {
+	mark(fn)
-	s := l.Slice()
-	for i := 0; i < len(s); i++ {
-		markbreak(labels, s[i], implicit)
-	}
 }
 
-// isterminating reports whether the Nodes list ends with a terminating statement.
+// isTermNodes reports whether the Nodes list ends with a terminating statement.
 func isTermNodes(l ir.Nodes) bool {
 	s := l.Slice()
 	c := len(s)
@@ -3723,7 +3724,7 @@ func isTermNodes(l ir.Nodes) bool {
 	return isTermNode(s[c-1])
 }
 
-// Isterminating reports whether the node n, the last one in a
+// isTermNode reports whether the node n, the last one in a
 // statement list, is a terminating statement.
 func isTermNode(n ir.Node) bool {
 	switch n.Op() {
@@ -3776,8 +3777,7 @@ func isTermNode(n ir.Node) bool {
 // checkreturn makes sure that fn terminates appropriately.
 func checkreturn(fn *ir.Func) {
 	if fn.Type().NumResults() != 0 && fn.Body().Len() != 0 {
-		var labels map[*types.Sym]ir.Node
+		markBreak(fn)
-		markbreaklist(&labels, fn.Body(), nil)
 		if !isTermNodes(fn.Body()) {
 			base.ErrorfAt(fn.Endlineno, "missing return at end of function")
 		}

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

@@ -3786,25 +3786,15 @@ func usefield(n ir.Node) {
 	Curfn.FieldTrack[sym] = struct{}{}
 }
 
-func candiscardlist(l ir.Nodes) bool {
+// hasSideEffects reports whether n contains any operations that could have observable side effects.
-	for _, n := range l.Slice() {
+func hasSideEffects(n ir.Node) bool {
-		if !candiscard(n) {
+	found := ir.Find(n, func(n ir.Node) interface{} {
-			return false
-		}
-	}
-	return true
-}
-
-func candiscard(n ir.Node) bool {
-	if n == nil {
-		return true
-	}
-
 		switch n.Op() {
+		// Assume side effects unless we know otherwise.
 		default:
-		return false
+			return n
 
-		// Discardable as long as the subpieces are.
+		// No side effects here (arguments are checked separately).
 		case ir.ONAME,
 			ir.ONONAME,
 			ir.OTYPE,
@@ -3858,35 +3848,29 @@ func candiscard(n ir.Node) bool {
 			ir.OREAL,
 			ir.OIMAG,
 			ir.OCOMPLEX:
-		break
+			return nil
 
-		// Discardable as long as we know it's not division by zero.
+		// Only possible side effect is division by zero.
 		case ir.ODIV, ir.OMOD:
-		if n.Right().Op() == ir.OLITERAL && constant.Sign(n.Right().Val()) != 0 {
+			if n.Right().Op() != ir.OLITERAL || constant.Sign(n.Right().Val()) == 0 {
-			break
+				return n
 			}
-		return false
 
-		// Discardable as long as we know it won't fail because of a bad size.
+		// Only possible side effect is panic on invalid size,
+		// but many makechan and makemap use size zero, which is definitely OK.
 		case ir.OMAKECHAN, ir.OMAKEMAP:
-		if ir.IsConst(n.Left(), constant.Int) && constant.Sign(n.Left().Val()) == 0 {
+			if !ir.IsConst(n.Left(), constant.Int) || constant.Sign(n.Left().Val()) != 0 {
-			break
+				return n
-		}
-		return false
-
-		// Difficult to tell what sizes are okay.
-	case ir.OMAKESLICE:
-		return false
-
-	case ir.OMAKESLICECOPY:
-		return false
 			}
 
-	if !candiscard(n.Left()) || !candiscard(n.Right()) || !candiscardlist(n.Init()) || !candiscardlist(n.Body()) || !candiscardlist(n.List()) || !candiscardlist(n.Rlist()) {
+		// Only possible side effect is panic on invalid size.
-		return false
+		// TODO(rsc): Merge with previous case (probably breaks toolstash -cmp).
+		case ir.OMAKESLICE, ir.OMAKESLICECOPY:
+			return n
 		}
-
+		return nil
-	return true
+	})
+	return found != nil
 }
 
 // Rewrite