From: Josh Bleecher Snyder Date: Thu, 27 Apr 2017 23:27:47 +0000 (-0700) Subject: cmd/compile: use a map to track liveness variable indices X-Git-Tag: go1.9beta1~403 X-Git-Url: http://www.git.cypherpunks.su/?a=commitdiff_plain;h=794d29a46f01b800b208bbc32f0ccb89b83c244d;p=gostls13.git 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 TryBot-Result: Gobot Gobot Reviewed-by: Matthew Dempsky --- diff --git a/src/cmd/compile/internal/gc/plive.go b/src/cmd/compile/internal/gc/plive.go index 3f2eb76c37..4811037311 100644 --- a/src/cmd/compile/internal/gc/plive.go +++ b/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) diff --git a/src/cmd/compile/internal/gc/syntax.go b/src/cmd/compile/internal/gc/syntax.go index 7c7f08653e..234ebad41c 100644 --- a/src/cmd/compile/internal/gc/syntax.go +++ b/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 "" + } return n.Func.Nname.Sym.Name }