cmd/compile: use a map to track liveness variable indices

It is not safe to modify Node.Opt in the backend.
Instead of using Node.Opt to store liveness variable indices, use a map.
This simplifies the code and makes it much more clearly race-free.
There are generally few such variables, so the maps are not a significant
source of allocations; this also remove some allocations from putting
int32s into interfaces.

Because map lookups are more expensive than interface value extraction,
reorder valueEffects to do the map lookup last.

The only remaining use of Node.Opt is now in esc.go.

Passes toolstash-check.

Fixes #20144

name        old alloc/op      new alloc/op      delta
Template         37.8MB ± 0%       37.9MB ± 0%    ~     (p=0.548 n=5+5)
Unicode          28.9MB ± 0%       28.9MB ± 0%    ~     (p=0.548 n=5+5)
GoTypes           110MB ± 0%        110MB ± 0%  +0.16%  (p=0.008 n=5+5)
Compiler          461MB ± 0%        462MB ± 0%  +0.08%  (p=0.008 n=5+5)
SSA              1.11GB ± 0%       1.11GB ± 0%  +0.11%  (p=0.008 n=5+5)
Flate            24.7MB ± 0%       24.7MB ± 0%    ~     (p=0.690 n=5+5)
GoParser         31.1MB ± 0%       31.1MB ± 0%    ~     (p=0.841 n=5+5)
Reflect          73.7MB ± 0%       73.8MB ± 0%  +0.23%  (p=0.008 n=5+5)
Tar              25.8MB ± 0%       25.7MB ± 0%    ~     (p=0.690 n=5+5)
XML              41.2MB ± 0%       41.2MB ± 0%    ~     (p=0.841 n=5+5)
[Geo mean]       71.9MB            71.9MB       +0.06%

name        old allocs/op     new allocs/op     delta
Template           385k ± 0%         384k ± 0%    ~     (p=0.548 n=5+5)
Unicode            344k ± 0%         343k ± 1%    ~     (p=0.421 n=5+5)
GoTypes           1.16M ± 0%        1.16M ± 0%    ~     (p=0.690 n=5+5)
Compiler          4.43M ± 0%        4.42M ± 0%    ~     (p=0.095 n=5+5)
SSA               9.86M ± 0%        9.84M ± 0%  -0.19%  (p=0.008 n=5+5)
Flate              238k ± 0%         238k ± 0%    ~     (p=1.000 n=5+5)
GoParser           321k ± 0%         320k ± 0%    ~     (p=0.310 n=5+5)
Reflect            956k ± 0%         956k ± 0%    ~     (p=1.000 n=5+5)
Tar                252k ± 0%         251k ± 0%    ~     (p=0.056 n=5+5)
XML                402k ± 1%         400k ± 1%  -0.57%  (p=0.032 n=5+5)
[Geo mean]         740k              739k       -0.19%

Change-Id: Id5916c9def76add272e89c59fe10968f0a6bb01d
Reviewed-on: https://go-review.googlesource.com/42135
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/internal/gc/plive.go

@@ -117,6 +117,7 @@ type Liveness struct {
 	fn         *Node
 	f          *ssa.Func
 	vars       []*Node
+	idx        map[*Node]int32
 	stkptrsize int64
 
 	be []BlockEffects
@@ -148,31 +149,20 @@ func livenessShouldTrack(n *Node) bool {
 	return n.Op == ONAME && (n.Class() == PAUTO || n.Class() == PPARAM || n.Class() == PPARAMOUT) && types.Haspointers(n.Type)
 }
 
-// getvariables returns the list of on-stack variables that we need to track.
-func getvariables(fn *Node) []*Node {
+// getvariables returns the list of on-stack variables that we need to track
+// and a map for looking up indices by *Node.
+func getvariables(fn *Node) ([]*Node, map[*Node]int32) {
 	var vars []*Node
 	for _, n := range fn.Func.Dcl {
-		if n.Op == ONAME {
-			// The Node.opt field is available for use by optimization passes.
-			// We use it to hold the index of the node in the variables array
-			// (nil means the Node is not in the variables array).
-			// The Node.curfn field is supposed to be set to the current function
-			// already, but for some compiler-introduced names it seems not to be,
-			// so fix that here.
-			// Later, when we want to find the index of a node in the variables list,
-			// we will check that n.Curfn == lv.fn and n.Opt() != nil. Then n.Opt().(int32)
-			// is the index in the variables list.
-			n.SetOpt(nil)
-			n.Name.Curfn = fn
-		}
-
 		if livenessShouldTrack(n) {
-			n.SetOpt(int32(len(vars)))
 			vars = append(vars, n)
 		}
 	}
-
-	return vars
+	idx := make(map[*Node]int32, len(vars))
+	for i, n := range vars {
+		idx[n] = int32(i)
+	}
+	return vars, idx
 }
 
 func (lv *Liveness) initcache() {
@@ -238,9 +228,9 @@ const (
 // valueEffects returns the index of a variable in lv.vars and the
 // liveness effects v has on that variable.
 // If v does not affect any tracked variables, it returns -1, 0.
-func (lv *Liveness) valueEffects(v *ssa.Value) (pos int32, effect liveEffect) {
+func (lv *Liveness) valueEffects(v *ssa.Value) (int32, liveEffect) {
 	n, e := affectedNode(v)
-	if e == 0 {
+	if e == 0 || n == nil || n.Op != ONAME { // cheapest checks first
 		return -1, 0
 	}
 
@@ -255,11 +245,7 @@ func (lv *Liveness) valueEffects(v *ssa.Value) (pos int32, effect liveEffect) {
 		}
 	}
 
-	pos = lv.liveIndex(n)
-	if pos < 0 {
-		return -1, 0
-	}
-
+	var effect liveEffect
 	if n.Addrtaken() {
 		if v.Op != ssa.OpVarKill {
 			effect |= avarinit
@@ -283,7 +269,14 @@ func (lv *Liveness) valueEffects(v *ssa.Value) (pos int32, effect liveEffect) {
 		}
 	}
 
-	return
+	if effect == 0 {
+		return -1, 0
+	}
+
+	if pos, ok := lv.idx[n]; ok {
+		return pos, effect
+	}
+	return -1, 0
 }
 
 // affectedNode returns the *Node affected by v
@@ -326,32 +319,15 @@ func affectedNode(v *ssa.Value) (*Node, ssa.SymEffect) {
 	return n, e
 }
 
-// liveIndex returns the index of n in the set of tracked vars.
-// If n is not a tracked var, liveIndex returns -1.
-// If n is not a tracked var but should be tracked, liveIndex crashes.
-func (lv *Liveness) liveIndex(n *Node) int32 {
-	if n == nil || n.Name.Curfn != lv.fn || !livenessShouldTrack(n) {
-		return -1
-	}
-
-	pos, ok := n.Opt().(int32) // index in vars
-	if !ok {
-		Fatalf("lost track of variable in liveness: %v (%p, %p)", n, n, n.Orig)
-	}
-	if pos >= int32(len(lv.vars)) || lv.vars[pos] != n {
-		Fatalf("bad bookkeeping in liveness: %v (%p, %p)", n, n, n.Orig)
-	}
-	return pos
-}
-
 // Constructs a new liveness structure used to hold the global state of the
 // liveness computation. The cfg argument is a slice of *BasicBlocks and the
 // vars argument is a slice of *Nodes.
-func newliveness(fn *Node, f *ssa.Func, vars []*Node, stkptrsize int64) *Liveness {
+func newliveness(fn *Node, f *ssa.Func, vars []*Node, idx map[*Node]int32, stkptrsize int64) *Liveness {
 	lv := &Liveness{
 		fn:         fn,
 		f:          f,
 		vars:       vars,
+		idx:        idx,
 		stkptrsize: stkptrsize,
 		be:         make([]BlockEffects, f.NumBlocks()),
 	}
@@ -1308,8 +1284,8 @@ func livenessemit(lv *Liveness, argssym, livesym *obj.LSym) {
 // Returns a map from GC safe points to their corresponding stack map index.
 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)
+	vars, idx := getvariables(e.curfn)
+	lv := newliveness(e.curfn, f, vars, idx, e.stkptrsize)
 
 	// Run the dataflow framework.
 	livenessprologue(lv)

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

@@ -214,6 +214,9 @@ func (n *Node) mayBeShared() bool {
 
 // funcname returns the name of the function n.
 func (n *Node) funcname() string {
+	if n == nil || n.Func == nil || n.Func.Nname == nil {
+		return "<nil>"
+	}
 	return n.Func.Nname.Sym.Name
 }