]> Cypherpunks repositories - gostls13.git/commitdiff
exp/ssa: reimplement logic for field selection.
authorAlan Donovan <adonovan@google.com>
Tue, 26 Feb 2013 18:32:22 +0000 (13:32 -0500)
committerAlan Donovan <adonovan@google.com>
Tue, 26 Feb 2013 18:32:22 +0000 (13:32 -0500)
The previous approach desugared the ast.SelectorExpr
to make implicit field selections explicit.  But:
1) it was clunky since it required allocating temporary
   syntax trees.
2) it was not thread-safe since it required poking
   types into the shared type map for the new ASTs.
3) the desugared syntax had no place to represent the
   package lexically enclosing each implicit field
   selection, so it was as if they all occurred in the
   same package as the explicit field selection.
   This meant unexported field names changed meaning.

This CL does what I should have done all along: just
generate the SSA instructions directly from the original
AST and the promoted field information.

Also:
- add logStack util for paired start/end log messages.
  Useful for debugging crashes.

R=gri
CC=golang-dev
https://golang.org/cl/7395052

src/pkg/exp/ssa/builder.go
src/pkg/exp/ssa/func.go
src/pkg/exp/ssa/promote.go
src/pkg/exp/ssa/ssa.go
src/pkg/exp/ssa/util.go

index 3d71a7a8de0c6b5db25a0141836090723ca3840c..d76d1ffa7513a24279ba2a10d73fed5924cd5740 100644 (file)
@@ -23,8 +23,6 @@ package ssa
 // The Builder's and Program's indices (maps) are populated and
 // mutated during the CREATE phase, but during the BUILD phase they
 // remain constant, with the following exceptions:
-// - demoteSelector mutates Builder.types during the BUILD phase.
-//   TODO(adonovan): fix: let's not do that.
 // - globalValueSpec mutates Builder.nTo1Vars.
 //   TODO(adonovan): make this a per-Package map so it's thread-safe.
 // - Program.methodSets is populated lazily across phases.
@@ -517,63 +515,100 @@ func (b *Builder) builtin(fn *Function, name string, args []ast.Expr, typ types.
        return nil // treat all others as a regular function call
 }
 
-// demoteSelector returns a SelectorExpr syntax tree that is
-// equivalent to sel but contains no selections of promoted fields.
-// It returns the field index of the explicit (=outermost) selection.
-//
-// pkg is the package in which the reference occurs.  This is
-// significant because a non-exported field is considered distinct
-// from a field of that name in any other package.
-//
-// This is a rather clunky and inefficient implementation, but it
-// (a) is simple and hopefully self-evidently correct and
-// (b) permits us to decouple the demotion from the code generation,
-// the latter being performed in two modes: addr() for lvalues,
-// expr() for rvalues.
-// It does require mutation of Builder.types though; if we want to
-// make the Builder concurrent we'll have to avoid that.
-// TODO(adonovan): opt: emit code directly rather than desugaring the AST.
-//
-func (b *Builder) demoteSelector(sel *ast.SelectorExpr, pkg *Package) (sel2 *ast.SelectorExpr, index int) {
-       id := makeId(sel.Sel.Name, pkg.Types)
-       xtype := b.exprType(sel.X)
-       // fmt.Fprintln(os.Stderr, xtype, id) // debugging
-       st := underlyingType(deref(xtype)).(*types.Struct)
+// selector evaluates the selector expression e and returns its value,
+// or if wantAddr is true, its address, in which case escaping
+// indicates whether the caller intends to use the resulting pointer
+// in a potentially escaping way.
+//
+func (b *Builder) selector(fn *Function, e *ast.SelectorExpr, wantAddr, escaping bool) Value {
+       id := makeId(e.Sel.Name, fn.Pkg.Types)
+       st := underlyingType(deref(b.exprType(e.X))).(*types.Struct)
+       index := -1
        for i, f := range st.Fields {
                if IdFromQualifiedName(f.QualifiedName) == id {
-                       return sel, i
+                       index = i
+                       break
+               }
+       }
+       var path *anonFieldPath
+       if index == -1 {
+               // Not a named field.  Use breadth-first algorithm.
+               path, index = findPromotedField(st, id)
+               if path == nil {
+                       panic("field not found, even with promotion: " + e.Sel.Name)
                }
        }
-       // Not a named field.  Use breadth-first algorithm.
-       path, index := findPromotedField(st, id)
-       if path == nil {
-               panic("field not found, even with promotion: " + sel.Sel.Name)
+       fieldType := b.exprType(e)
+       if wantAddr {
+               return b.fieldAddr(fn, e.X, path, index, fieldType, escaping)
        }
+       return b.fieldExpr(fn, e.X, path, index, fieldType)
+}
 
-       // makeSelector(e, [C,B,A]) returns (((e.A).B).C).
-       // e is the original selector's base.
-       // This function has no free variables.
-       var makeSelector func(b *Builder, e ast.Expr, path *anonFieldPath) *ast.SelectorExpr
-       makeSelector = func(b *Builder, e ast.Expr, path *anonFieldPath) *ast.SelectorExpr {
-               x := e
-               if path.tail != nil {
-                       x = makeSelector(b, e, path.tail)
+// fieldAddr evaluates the base expression (a struct or *struct),
+// applies to it any implicit field selections from path, and then
+// selects the field #index of type fieldType.
+// Its address is returned.
+//
+// (fieldType can be derived from base+index.)
+//
+func (b *Builder) fieldAddr(fn *Function, base ast.Expr, path *anonFieldPath, index int, fieldType types.Type, escaping bool) Value {
+       var x Value
+       if path != nil {
+               switch underlyingType(path.field.Type).(type) {
+               case *types.Struct:
+                       x = b.fieldAddr(fn, base, path.tail, path.index, path.field.Type, escaping)
+               case *types.Pointer:
+                       x = b.fieldExpr(fn, base, path.tail, path.index, path.field.Type)
                }
-               sel := &ast.SelectorExpr{
-                       X:   x,
-                       Sel: &ast.Ident{Name: path.field.Name},
+       } else {
+               switch underlyingType(b.exprType(base)).(type) {
+               case *types.Struct:
+                       x = b.addr(fn, base, escaping).(address).addr
+               case *types.Pointer:
+                       x = b.expr(fn, base)
                }
-               b.types[sel] = path.field.Type // TODO(adonovan): opt: not thread-safe
-               return sel
        }
+       v := &FieldAddr{
+               X:     x,
+               Field: index,
+       }
+       v.setType(pointer(fieldType))
+       return fn.emit(v)
+}
 
-       // Construct new SelectorExpr, bottom up.
-       sel2 = &ast.SelectorExpr{
-               X:   makeSelector(b, sel.X, path),
-               Sel: sel.Sel,
+// fieldExpr evaluates the base expression (a struct or *struct),
+// applies to it any implicit field selections from path, and then
+// selects the field #index of type fieldType.
+// Its value is returned.
+//
+// (fieldType can be derived from base+index.)
+//
+func (b *Builder) fieldExpr(fn *Function, base ast.Expr, path *anonFieldPath, index int, fieldType types.Type) Value {
+       var x Value
+       if path != nil {
+               x = b.fieldExpr(fn, base, path.tail, path.index, path.field.Type)
+       } else {
+               x = b.expr(fn, base)
        }
-       b.types[sel2] = b.exprType(sel) // TODO(adonovan): opt: not thread-safe
-       return
+       switch underlyingType(x.Type()).(type) {
+       case *types.Struct:
+               v := &Field{
+                       X:     x,
+                       Field: index,
+               }
+               v.setType(fieldType)
+               return fn.emit(v)
+
+       case *types.Pointer: // *struct
+               v := &FieldAddr{
+                       X:     x,
+                       Field: index,
+               }
+               v.setType(pointer(fieldType))
+               return emitLoad(fn, fn.emit(v))
+       }
+       panic("unreachable")
 }
 
 // addr lowers a single-result addressable expression e to SSA form,
@@ -633,20 +668,7 @@ func (b *Builder) addr(fn *Function, e ast.Expr, escaping bool) lvalue {
                }
 
                // e.f where e is an expression.
-               e, index := b.demoteSelector(e, fn.Pkg)
-               var x Value
-               switch underlyingType(b.exprType(e.X)).(type) {
-               case *types.Struct:
-                       x = b.addr(fn, e.X, escaping).(address).addr
-               case *types.Pointer:
-                       x = b.expr(fn, e.X)
-               }
-               v := &FieldAddr{
-                       X:     x,
-                       Field: index,
-               }
-               v.setType(pointer(b.exprType(e)))
-               return address{fn.emit(v)}
+               return address{b.selector(fn, e, true, escaping)}
 
        case *ast.IndexExpr:
                var x Value
@@ -867,33 +889,19 @@ func (b *Builder) expr(fn *Function, e ast.Expr) Value {
                }
 
                // (*T).f or T.f, the method f from the method-set of type T.
+               xtype := b.exprType(e.X)
                if b.isType(e.X) {
                        id := makeId(e.Sel.Name, fn.Pkg.Types)
-                       typ := b.exprType(e.X)
-                       if m := b.Prog.MethodSet(typ)[id]; m != nil {
+                       if m := b.Prog.MethodSet(xtype)[id]; m != nil {
                                return m
                        }
 
                        // T must be an interface; return method thunk.
-                       return makeImethodThunk(b.Prog, typ, id)
+                       return makeImethodThunk(b.Prog, xtype, id)
                }
 
                // e.f where e is an expression.
-               e, index := b.demoteSelector(e, fn.Pkg)
-               switch underlyingType(b.exprType(e.X)).(type) {
-               case *types.Struct:
-                       // Non-addressable struct in a register.
-                       v := &Field{
-                               X:     b.expr(fn, e.X),
-                               Field: index,
-                       }
-                       v.setType(b.exprType(e))
-                       return fn.emit(v)
-
-               case *types.Pointer: // *struct
-                       // Addressable structs; use FieldAddr and Load.
-                       return b.addr(fn, e, false).load(fn)
-               }
+               return b.selector(fn, e, false, false)
 
        case *ast.IndexExpr:
                switch t := underlyingType(b.exprType(e.X)).(type) {
@@ -1338,7 +1346,7 @@ func (b *Builder) globalValueSpec(init *Function, spec *ast.ValueSpec, g *Global
                if !b.nTo1Vars[spec] {
                        b.nTo1Vars[spec] = true
                        if b.mode&LogSource != 0 {
-                               fmt.Fprintln(os.Stderr, "build globals", spec.Names) // ugly...
+                               defer logStack("build globals %s", spec.Names)()
                        }
                        tuple := b.exprN(init, spec.Values[0])
                        rtypes := tuple.Type().(*types.Result).Values
@@ -2287,6 +2295,10 @@ start:
                }
                var results []Value
                // Per the spec, there are three distinct cases of return.
+               // TODO(adonovan): fix: the design of Ret is incorrect:
+               // deferred procedures may modify named result locations
+               // after "Ret" has loaded its operands, causing the calls's
+               // result to change.  Tricky... rethink.
                switch {
                case len(s.Results) == 0:
                        // Return with no arguments.
@@ -2412,6 +2424,10 @@ func (b *Builder) buildFunction(fn *Function) {
        if fn.syntax.body == nil {
                return // Go source function with no body (external)
        }
+       if fn.Prog.mode&LogSource != 0 {
+               defer logStack("build function %s @ %s",
+                       fn.FullName(), fn.Prog.Files.Position(fn.Pos))()
+       }
        fn.start(b.idents)
        b.stmt(fn, fn.syntax.body)
        if cb := fn.currentBlock; cb != nil && (cb == fn.Blocks[0] || cb.Preds != nil) {
@@ -2676,7 +2692,8 @@ func (b *Builder) buildDecl(pkg *Package, decl ast.Decl) {
                } else if decl.Recv == nil && id.Name == "init" {
                        // init() block
                        if b.mode&LogSource != 0 {
-                               fmt.Fprintln(os.Stderr, "build init block @", b.Prog.Files.Position(decl.Pos()))
+                               fmt.Fprintln(os.Stderr, "build init block @",
+                                       b.Prog.Files.Position(decl.Pos()))
                        }
                        init := pkg.Init
 
@@ -2711,7 +2728,7 @@ func (b *Builder) BuildPackage(p *Package) {
                return // already done (or nothing to do)
        }
        if b.mode&LogSource != 0 {
-               fmt.Fprintln(os.Stderr, "build package", p.Types.Path)
+               defer logStack("build package %s", p.Types.Path)()
        }
        init := p.Init
        init.start(b.idents)
index 423ae6598468dcac7e50d4e819485147aaa0af29..eb45ee0f82276b59988a84106593e67e283d4659 100644 (file)
@@ -195,9 +195,6 @@ func (f *Function) addSpilledParam(obj types.Object) {
 // functions is skipped.
 //
 func (f *Function) start(idents map[*ast.Ident]types.Object) {
-       if f.Prog.mode&LogSource != 0 {
-               fmt.Fprintf(os.Stderr, "build function %s @ %s\n", f.FullName(), f.Prog.Files.Position(f.Pos))
-       }
        f.currentBlock = f.newBasicBlock("entry")
        f.objects = make(map[types.Object]Value) // needed for some synthetics, e.g. init
        if f.syntax == nil {
@@ -327,9 +324,6 @@ func (f *Function) finish() {
        if f.Prog.mode&SanityCheckFunctions != 0 {
                MustSanityCheck(f, nil)
        }
-       if f.Prog.mode&LogSource != 0 {
-               fmt.Fprintf(os.Stderr, "build function %s done\n", f.FullName())
-       }
 }
 
 // removeNilBlocks eliminates nils from f.Blocks and updates each
index acaf8921f5f2a9d9887b351c53c6af1dda553016..75979e0ab3328a8d4b6bc643ee0b8cfc6aab0223 100644 (file)
@@ -263,7 +263,7 @@ func makeBridgeMethod(prog *Program, typ types.Type, cand *candidate) *Function
        sig.Recv = &types.Var{Name: "recv", Type: typ}
 
        if prog.mode&LogSource != 0 {
-               fmt.Fprintf(os.Stderr, "makeBridgeMethod %s, %s, type %s\n", typ, cand, &sig)
+               defer logStack("makeBridgeMethod %s, %s, type %s", typ, cand, &sig)()
        }
 
        fn := &Function{
@@ -361,7 +361,7 @@ func makeBridgeMethod(prog *Program, typ types.Type, cand *candidate) *Function
 //
 func makeImethodThunk(prog *Program, typ types.Type, id Id) *Function {
        if prog.mode&LogSource != 0 {
-               fmt.Fprintf(os.Stderr, "makeImethodThunk %s.%s\n", typ, id)
+               defer logStack("makeImethodThunk %s.%s", typ, id)()
        }
        itf := underlyingType(typ).(*types.Interface)
        index, meth := methodIndex(itf, itf.Methods, id)
index a07153575075e4cf26b0719307161d6bb1d3ddc0..aba86ffa9b1ab65744a95da6d971c8bdf9b0aba6 100644 (file)
@@ -945,7 +945,7 @@ type Register struct {
        referrers []Instruction
 }
 
-// AnInstruction is a mix-in embedded by all Instructions.
+// anInstruction is a mix-in embedded by all Instructions.
 // It provides the implementations of the Block and SetBlock methods.
 type anInstruction struct {
        Block_ *BasicBlock // the basic block of this instruction
index a335e94e9262ac5321a27975c13353142a00eff9..15c03d44624ee64012795069b2d8b684eb91735e 100644 (file)
@@ -6,6 +6,8 @@ import (
        "fmt"
        "go/ast"
        "go/types"
+       "io"
+       "os"
        "reflect"
 )
 
@@ -208,3 +210,18 @@ func (p ids) Less(i, j int) bool {
                x.Pkg == y.Pkg && x.Name < y.Name
 }
 func (p ids) Swap(i, j int) { p[i], p[j] = p[j], p[i] }
+
+// logStack prints the formatted "start" message to stderr and
+// returns a closure that prints the corresponding "end" message.
+// Call using 'defer logStack(...)()' to show builder stack on panic.
+// Don't forget trailing parens!
+//
+func logStack(format string, args ...interface{}) func() {
+       msg := fmt.Sprintf(format, args...)
+       io.WriteString(os.Stderr, msg)
+       io.WriteString(os.Stderr, "\n")
+       return func() {
+               io.WriteString(os.Stderr, msg)
+               io.WriteString(os.Stderr, " end\n")
+       }
+}