]> Cypherpunks repositories - gostls13.git/commitdiff
cmd/compile: use a map to track liveness variable indices
authorJosh Bleecher Snyder <josharian@gmail.com>
Thu, 27 Apr 2017 23:27:47 +0000 (16:27 -0700)
committerJosh Bleecher Snyder <josharian@gmail.com>
Fri, 28 Apr 2017 19:50:53 +0000 (19:50 +0000)
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
src/cmd/compile/internal/gc/syntax.go

index 3f2eb76c379eb8e1e51e55736a558ba8a520b939..4811037311bd05bf8bd9eff165a66492eb9c61fa 100644 (file)
@@ -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)
index 7c7f08653e3e5e6cc24e6a0a6edd8b5be66d8b25..234ebad41c62b8cc613ba0d6d77cc0f4e0283255 100644 (file)
@@ -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
 }