]> Cypherpunks repositories - gostls13.git/commitdiff
[dev.ssa] cmd/compile: optimize phi ops
authorKeith Randall <khr@golang.org>
Fri, 15 Jan 2016 00:02:23 +0000 (16:02 -0800)
committerKeith Randall <khr@golang.org>
Wed, 20 Jan 2016 21:45:37 +0000 (21:45 +0000)
Redo how we keep track of forward references when building SSA.
When the forward reference is resolved, update the Value node
in place.

Improve the phi elimination pass so it can simplify phis of phis.

Give SSA package access to decoded line numbers.  Fix line numbers
for constant booleans.

Change-Id: I3dc9896148d260be2f3dd14cbe5db639ec9fa6b7
Reviewed-on: https://go-review.googlesource.com/18674
Reviewed-by: David Chase <drchase@google.com>
Run-TryBot: Keith Randall <khr@golang.org>

src/cmd/compile/internal/gc/ssa.go
src/cmd/compile/internal/ssa/compile.go
src/cmd/compile/internal/ssa/config.go
src/cmd/compile/internal/ssa/export_test.go
src/cmd/compile/internal/ssa/gen/genericOps.go
src/cmd/compile/internal/ssa/opGen.go
src/cmd/compile/internal/ssa/phielim.go
src/cmd/compile/internal/ssa/print.go

index be9af60bd8ec3ac0227ac6ee28705462f93e5309..42f484d5f2c8b331c02db15eef2cc336ded7cf17 100644 (file)
@@ -139,11 +139,6 @@ func buildssa(fn *Node) *ssa.Func {
                }
        }()
 
-       // We construct SSA using an algorithm similar to
-       // Brau, Buchwald, Hack, Leißa, Mallon, and Zwinkau
-       // http://pp.info.uni-karlsruhe.de/uploads/publikationen/braun13cc.pdf
-       // TODO: check this comment
-
        // Allocate starting block
        s.f.Entry = s.f.NewBlock(ssa.BlockPlain)
 
@@ -285,6 +280,9 @@ type state struct {
        // list of panic calls by function name and line number.
        // Used to deduplicate panic calls.
        panics map[funcLine]*ssa.Block
+
+       // list of FwdRef values.
+       fwdRefs []*ssa.Value
 }
 
 type funcLine struct {
@@ -1327,7 +1325,14 @@ func (s *state) expr(n *Node) *ssa.Value {
                case CTSTR:
                        return s.entryNewValue0A(ssa.OpConstString, n.Type, n.Val().U)
                case CTBOOL:
-                       return s.constBool(n.Val().U.(bool))
+                       v := s.constBool(n.Val().U.(bool))
+                       // For some reason the frontend gets the line numbers of
+                       // CTBOOL literals totally wrong.  Fix it here by grabbing
+                       // the line number of the enclosing AST node.
+                       if len(s.line) >= 2 {
+                               v.Line = s.line[len(s.line)-2]
+                       }
+                       return v
                case CTNIL:
                        t := n.Type
                        switch {
@@ -3172,9 +3177,10 @@ func (s *state) checkgoto(from *Node, to *Node) {
 func (s *state) variable(name *Node, t ssa.Type) *ssa.Value {
        v := s.vars[name]
        if v == nil {
-               // TODO: get type?  Take Sym as arg?
                v = s.newValue0A(ssa.OpFwdRef, t, name)
+               s.fwdRefs = append(s.fwdRefs, v)
                s.vars[name] = v
+               s.addNamedValue(name, v)
        }
        return v
 }
@@ -3184,40 +3190,38 @@ func (s *state) mem() *ssa.Value {
 }
 
 func (s *state) linkForwardReferences() {
-       // Build ssa graph.  Each variable on its first use in a basic block
+       // Build SSA graph.  Each variable on its first use in a basic block
        // leaves a FwdRef in that block representing the incoming value
        // of that variable.  This function links that ref up with possible definitions,
        // inserting Phi values as needed.  This is essentially the algorithm
-       // described by Brau, Buchwald, Hack, Leißa, Mallon, and Zwinkau:
+       // described by Braun, Buchwald, Hack, Leißa, Mallon, and Zwinkau:
        // http://pp.info.uni-karlsruhe.de/uploads/publikationen/braun13cc.pdf
-       for _, b := range s.f.Blocks {
-               for _, v := range b.Values {
-                       if v.Op != ssa.OpFwdRef {
-                               continue
-                       }
-                       name := v.Aux.(*Node)
-                       v.Op = ssa.OpCopy
-                       v.Aux = nil
-                       v.SetArgs1(s.lookupVarIncoming(b, v.Type, name))
-               }
+       // Differences:
+       //   - We use FwdRef nodes to postpone phi building until the CFG is
+       //     completely built.  That way we can avoid the notion of "sealed"
+       //     blocks.
+       //   - Phi optimization is a separate pass (in ../ssa/phielim.go).
+       for len(s.fwdRefs) > 0 {
+               v := s.fwdRefs[len(s.fwdRefs)-1]
+               s.fwdRefs = s.fwdRefs[:len(s.fwdRefs)-1]
+               s.resolveFwdRef(v)
        }
 }
 
-// lookupVarIncoming finds the variable's value at the start of block b.
-func (s *state) lookupVarIncoming(b *ssa.Block, t ssa.Type, name *Node) *ssa.Value {
-       // TODO(khr): have lookupVarIncoming overwrite the fwdRef or copy it
-       // will be used in, instead of having the result used in a copy value.
+// resolveFwdRef modifies v to be the variable's value at the start of its block.
+// v must be a FwdRef op.
+func (s *state) resolveFwdRef(v *ssa.Value) {
+       b := v.Block
+       name := v.Aux.(*Node)
+       v.Aux = nil
        if b == s.f.Entry {
-               if name == &memVar {
-                       return s.startmem
-               }
+               // Live variable at start of function.
                if canSSA(name) {
-                       v := s.entryNewValue0A(ssa.OpArg, t, name)
-                       // v starts with AuxInt == 0.
-                       s.addNamedValue(name, v)
-                       return v
+                       v.Op = ssa.OpArg
+                       v.Aux = name
+                       return
                }
-               // variable is live at the entry block.  Load it.
+               // Not SSAable.  Load it.
                addr := s.decladdrs[name]
                if addr == nil {
                        // TODO: closure args reach here.
@@ -3226,64 +3230,69 @@ func (s *state) lookupVarIncoming(b *ssa.Block, t ssa.Type, name *Node) *ssa.Val
                if _, ok := addr.Aux.(*ssa.ArgSymbol); !ok {
                        s.Fatalf("variable live at start of function %s is not an argument %s", b.Func.Name, name)
                }
-               return s.entryNewValue2(ssa.OpLoad, t, addr, s.startmem)
+               v.Op = ssa.OpLoad
+               v.AddArgs(addr, s.startmem)
+               return
+       }
+       if len(b.Preds) == 0 {
+               // This block is dead; we have no predecessors and we're not the entry block.
+               // It doesn't matter what we use here as long as it is well-formed.
+               v.Op = ssa.OpUnknown
+               return
        }
-       var vals []*ssa.Value
+       // Find variable value on each predecessor.
+       var argstore [4]*ssa.Value
+       args := argstore[:0]
        for _, p := range b.Preds {
-               vals = append(vals, s.lookupVarOutgoing(p, t, name))
+               args = append(args, s.lookupVarOutgoing(p, v.Type, name, v.Line))
        }
-       if len(vals) == 0 {
-               // This block is dead; we have no predecessors and we're not the entry block.
-               // It doesn't matter what we use here as long as it is well-formed,
-               // so use the default/zero value.
-               if name == &memVar {
-                       return s.startmem
+
+       // Decide if we need a phi or not.  We need a phi if there
+       // are two different args (which are both not v).
+       var w *ssa.Value
+       for _, a := range args {
+               if a == v {
+                       continue // self-reference
                }
-               return s.zeroVal(name.Type)
-       }
-       v0 := vals[0]
-       for i := 1; i < len(vals); i++ {
-               if vals[i] != v0 {
-                       // need a phi value
-                       v := b.NewValue0(s.peekLine(), ssa.OpPhi, t)
-                       v.AddArgs(vals...)
-                       s.addNamedValue(name, v)
-                       return v
+               if a == w {
+                       continue // already have this witness
+               }
+               if w != nil {
+                       // two witnesses, need a phi value
+                       v.Op = ssa.OpPhi
+                       v.AddArgs(args...)
+                       return
                }
+               w = a // save witness
+       }
+       if w == nil {
+               s.Fatalf("no witness for reachable phi %s", v)
        }
-       return v0
+       // One witness.  Make v a copy of w.
+       v.Op = ssa.OpCopy
+       v.AddArg(w)
 }
 
 // lookupVarOutgoing finds the variable's value at the end of block b.
-func (s *state) lookupVarOutgoing(b *ssa.Block, t ssa.Type, name *Node) *ssa.Value {
+func (s *state) lookupVarOutgoing(b *ssa.Block, t ssa.Type, name *Node, line int32) *ssa.Value {
        m := s.defvars[b.ID]
        if v, ok := m[name]; ok {
                return v
        }
        // The variable is not defined by b and we haven't
-       // looked it up yet.  Generate v, a copy value which
-       // will be the outgoing value of the variable.  Then
-       // look up w, the incoming value of the variable.
-       // Make v = copy(w).  We need the extra copy to
-       // prevent infinite recursion when looking up the
-       // incoming value of the variable.
-       v := b.NewValue0(s.peekLine(), ssa.OpCopy, t)
+       // looked it up yet.  Generate a FwdRef for the variable and return that.
+       v := b.NewValue0A(line, ssa.OpFwdRef, t, name)
+       s.fwdRefs = append(s.fwdRefs, v)
        m[name] = v
-       v.AddArg(s.lookupVarIncoming(b, t, name))
+       s.addNamedValue(name, v)
        return v
 }
 
-// TODO: the above mutually recursive functions can lead to very deep stacks.  Fix that.
-
 func (s *state) addNamedValue(n *Node, v *ssa.Value) {
        if n.Class == Pxxx {
                // Don't track our dummy nodes (&memVar etc.).
                return
        }
-       if n.Sym == nil {
-               // TODO: What the heck is this?
-               return
-       }
        if strings.HasPrefix(n.Sym.Name, "autotmp_") {
                // Don't track autotmp_ variables.
                return
@@ -3910,7 +3919,7 @@ func (s *genState) genValue(v *ssa.Value) {
                p.To.Sym = Linksym(Pkglookup("duffcopy", Runtimepkg))
                p.To.Offset = v.AuxInt
 
-       case ssa.OpCopy, ssa.OpAMD64MOVQconvert: // TODO: lower Copy to MOVQ earlier?
+       case ssa.OpCopy, ssa.OpAMD64MOVQconvert: // TODO: use MOVQreg for reg->reg copies instead of OpCopy?
                if v.Type.IsMemory() {
                        return
                }
@@ -3970,12 +3979,6 @@ func (s *genState) genValue(v *ssa.Value) {
                                v.Fatalf("phi arg at different location than phi: %v @ %v, but arg %v @ %v\n%s\n", v, loc, a, aloc, v.Block.Func)
                        }
                }
-       case ssa.OpConst8, ssa.OpConst16, ssa.OpConst32, ssa.OpConst64, ssa.OpConstString, ssa.OpConstNil, ssa.OpConstBool,
-               ssa.OpConst32F, ssa.OpConst64F:
-               if v.Block.Func.RegAlloc[v.ID] != nil {
-                       v.Fatalf("const value %v shouldn't have a location", v)
-               }
-
        case ssa.OpInitMem:
                // memory arg needs no code
        case ssa.OpArg:
@@ -4596,6 +4599,10 @@ func (e *ssaExport) CanSSA(t ssa.Type) bool {
        return canSSAType(t.(*Type))
 }
 
+func (e *ssaExport) Line(line int32) string {
+       return Ctxt.Line(int(line))
+}
+
 // Log logs a message from the compiler.
 func (e *ssaExport) Logf(msg string, args ...interface{}) {
        // If e was marked as unimplemented, anything could happen. Ignore.
index 20af6fd5bdb3b84b281d5504cdd70a17dfc54440..64c1412f9d5cb17e12580b26307768de23141f43 100644 (file)
@@ -81,8 +81,9 @@ type pass struct {
 
 // list of passes for the compiler
 var passes = [...]pass{
-       {"phielim", phielim},
-       {"copyelim", copyelim},
+       // TODO: combine phielim and copyelim into a single pass?
+       {"early phielim", phielim},
+       {"early copyelim", copyelim},
        {"early deadcode", deadcode}, // remove generated dead code to avoid doing pointless work during opt
        {"decompose", decompose},
        {"opt", opt},
@@ -97,6 +98,9 @@ var passes = [...]pass{
        {"lowered cse", cse},
        {"lowered deadcode", deadcode},
        {"checkLower", checkLower},
+       {"late phielim", phielim},
+       {"late copyelim", copyelim},
+       {"late deadcode", deadcode},
        {"critical", critical},   // remove critical edges
        {"layout", layout},       // schedule blocks
        {"schedule", schedule},   // schedule values
index 7ef2fbd2fcdbd0ebc11d73ceb8ba90bb604536b0..fb0d886b88f5d6b1d2b75fa963afbb39f8f005f9 100644 (file)
@@ -67,6 +67,9 @@ type Frontend interface {
        // Auto returns a Node for an auto variable of the given type.
        // The SSA compiler uses this function to allocate space for spills.
        Auto(Type) GCNode
+
+       // Line returns a string describing the given line number.
+       Line(int32) string
 }
 
 // interface used to hold *gc.Node.  We'd use *gc.Node directly but
index f4d8d58549d7c22b285c9b74634de919d63c1c68..badafadd7090924422c2c03b29d6298202b2f37d 100644 (file)
@@ -31,6 +31,9 @@ func (DummyFrontend) StringData(s string) interface{} {
 func (DummyFrontend) Auto(t Type) GCNode {
        return nil
 }
+func (DummyFrontend) Line(line int32) string {
+       return "unknown.go:0"
+}
 
 func (d DummyFrontend) Logf(msg string, args ...interface{}) { d.t.Logf(msg, args...) }
 
index d17f5589785183bf7c15208162c03cff1cbdf7de..5c1a7af3638f462f3b50de1391c049ae2caf5451 100644 (file)
@@ -371,6 +371,9 @@ var genericOps = []opData{
        // Used during ssa construction.  Like Copy, but the arg has not been specified yet.
        {name: "FwdRef"},
 
+       // Unknown value.  Used for Values whose values don't matter because they are dead code.
+       {name: "Unknown"},
+
        {name: "VarDef", typ: "Mem"}, // aux is a *gc.Node of a variable that is about to be initialized.  arg0=mem, returns mem
        {name: "VarKill"},            // aux is a *gc.Node of a variable that is known to be dead.  arg0=mem, returns mem
        {name: "VarLive"},            // aux is a *gc.Node of a variable that must be kept live.  arg0=mem, returns mem
index 433794a03b5d1fc9e7ccea2d8cccda7ea2e42e06..e3fc8aba3bd33cc487efdfcec71193efe8d3649a 100644 (file)
@@ -550,6 +550,7 @@ const (
        OpStoreReg
        OpLoadReg
        OpFwdRef
+       OpUnknown
        OpVarDef
        OpVarKill
        OpVarLive
@@ -4303,6 +4304,10 @@ var opcodeTable = [...]opInfo{
                name:    "FwdRef",
                generic: true,
        },
+       {
+               name:    "Unknown",
+               generic: true,
+       },
        {
                name:    "VarDef",
                generic: true,
index be9503248b5bfcde33d951728d67e8e7346369e4..aaa0a0f2386b1f647c5cfacd50b1c0da610944ea 100644 (file)
@@ -10,29 +10,52 @@ package ssa
 // these phis are redundant:
 //   v = phi(x,x,x)
 //   v = phi(x,v,x,v)
+// We repeat this process to also catch situations like:
+//   v = phi(x, phi(x, x), phi(x, v))
+// TODO: Can we also simplify cases like:
+//   v = phi(v, w, x)
+//   w = phi(v, w, x)
+// and would that be useful?
 func phielim(f *Func) {
-       argSet := newSparseSet(f.NumValues())
-       var args []*Value
-       for _, b := range f.Blocks {
-               for _, v := range b.Values {
-                       if v.Op != OpPhi {
-                               continue
-                       }
-                       argSet.clear()
-                       args = args[:0]
-                       for _, x := range v.Args {
-                               for x.Op == OpCopy {
-                                       x = x.Args[0]
+       for {
+               changed := false
+               for _, b := range f.Blocks {
+               nextv:
+                       for _, v := range b.Values {
+                               if v.Op != OpPhi {
+                                       continue
                                }
-                               if x != v && !argSet.contains(x.ID) {
-                                       argSet.add(x.ID)
-                                       args = append(args, x)
+                               // If there are two distinct args of v which
+                               // are not v itself, then the phi must remain.
+                               // Otherwise, we can replace it with a copy.
+                               var w *Value
+                               for _, x := range v.Args {
+                                       for x.Op == OpCopy {
+                                               x = x.Args[0]
+                                       }
+                                       if x == v {
+                                               continue
+                                       }
+                                       if x == w {
+                                               continue
+                                       }
+                                       if w != nil {
+                                               continue nextv
+                                       }
+                                       w = x
+                               }
+                               if w == nil {
+                                       // v references only itself.  It must be in
+                                       // a dead code loop.  Don't bother modifying it.
+                                       continue
                                }
-                       }
-                       if len(args) == 1 {
                                v.Op = OpCopy
-                               v.SetArgs1(args[0])
+                               v.SetArgs1(w)
+                               changed = true
                        }
                }
+               if !changed {
+                       break
+               }
        }
 }
index b61e6f1cc7ee09d9e52d0f9df56b1237107bbb44..c6f84ab6cbac8027f45ed34588115ad98127d08d 100644 (file)
@@ -61,6 +61,8 @@ func (p stringFuncPrinter) endBlock(b *Block) {
 
 func (p stringFuncPrinter) value(v *Value, live bool) {
        fmt.Fprint(p.w, "    ")
+       //fmt.Fprint(p.w, v.Block.Func.Config.fe.Line(v.Line))
+       //fmt.Fprint(p.w, ": ")
        fmt.Fprint(p.w, v.LongString())
        if !live {
                fmt.Fprint(p.w, " DEAD")