From 562a199961c28741a93cf7a0365c0646da3ddb9f Mon Sep 17 00:00:00 2001 From: Matthew Dempsky Date: Wed, 4 Apr 2018 15:53:27 -0700 Subject: [PATCH] cmd/compile: extract inline related fields into separate Inline type Inl, Inldcl, and InlCost are only applicable to functions with bodies that can be inlined, so pull them out into a separate Inline type to make understanding them easier. A side benefit is that we can check if a function can be inlined by just checking if n.Func.Inl is non-nil, which simplifies handling of empty function bodies. While here, remove some unnecessary Curfn twiddling, and make imported functions use Inl.Dcl instead of Func.Dcl for consistency for local functions. Passes toolstash-check. Change-Id: Ifd4a80349d85d9e8e4484952b38ec4a63182e81f Reviewed-on: https://go-review.googlesource.com/104756 Run-TryBot: Matthew Dempsky TryBot-Result: Gobot Gobot Reviewed-by: Robert Griesemer --- src/cmd/compile/internal/gc/bexport.go | 6 +- src/cmd/compile/internal/gc/bimport.go | 22 +++----- src/cmd/compile/internal/gc/inl.go | 66 ++++++++++------------ src/cmd/compile/internal/gc/main.go | 2 +- src/cmd/compile/internal/gc/pgen.go | 9 +-- src/cmd/compile/internal/gc/sizeof_test.go | 2 +- src/cmd/compile/internal/gc/syntax.go | 18 +++++- 7 files changed, 61 insertions(+), 64 deletions(-) diff --git a/src/cmd/compile/internal/gc/bexport.go b/src/cmd/compile/internal/gc/bexport.go index 982f11fb88..060e7b7a67 100644 --- a/src/cmd/compile/internal/gc/bexport.go +++ b/src/cmd/compile/internal/gc/bexport.go @@ -377,11 +377,11 @@ func export(out *bufio.Writer, trace bool) int { // function has inlineable body: // write index and body if p.trace { - p.tracef("\n----\nfunc { %#v }\n", f.Inl) + p.tracef("\n----\nfunc { %#v }\n", asNodes(f.Inl.Body)) } p.int(i) - p.int(int(f.InlCost)) - p.stmtList(f.Inl) + p.int(int(f.Inl.Cost)) + p.stmtList(asNodes(f.Inl.Body)) if p.trace { p.tracef("\n") } diff --git a/src/cmd/compile/internal/gc/bimport.go b/src/cmd/compile/internal/gc/bimport.go index 176da7f759..01e77ef859 100644 --- a/src/cmd/compile/internal/gc/bimport.go +++ b/src/cmd/compile/internal/gc/bimport.go @@ -188,28 +188,22 @@ func Import(imp *types.Pkg, in *bufio.Reader) { // parameter renaming which doesn't matter if we don't have a body. inlCost := p.int() - if f := p.funcList[i]; f != nil && f.Func.Inl.Len() == 0 { + if f := p.funcList[i]; f != nil && f.Func.Inl == nil { // function not yet imported - read body and set it funchdr(f) body := p.stmtList() - if body == nil { - // Make sure empty body is not interpreted as - // no inlineable body (see also parser.fnbody) - // (not doing so can cause significant performance - // degradation due to unnecessary calls to empty - // functions). - body = []*Node{nod(OEMPTY, nil, nil)} + funcbody() + f.Func.Inl = &Inline{ + Cost: int32(inlCost), + Body: body, } - f.Func.Inl.Set(body) - f.Func.InlCost = int32(inlCost) - if Debug['E'] > 0 && Debug['m'] > 2 && f.Func.Inl.Len() != 0 { + if Debug['E'] > 0 && Debug['m'] > 2 { if Debug['m'] > 3 { - fmt.Printf("inl body for %v: %+v\n", f, f.Func.Inl) + fmt.Printf("inl body for %v: %+v\n", f, asNodes(body)) } else { - fmt.Printf("inl body for %v: %v\n", f, f.Func.Inl) + fmt.Printf("inl body for %v: %v\n", f, asNodes(body)) } } - funcbody() } else { // function already imported - read body but discard declarations dclcontext = PDISCARD // throw away any declarations diff --git a/src/cmd/compile/internal/gc/inl.go b/src/cmd/compile/internal/gc/inl.go index 54c031178c..9ee6176ead 100644 --- a/src/cmd/compile/internal/gc/inl.go +++ b/src/cmd/compile/internal/gc/inl.go @@ -70,7 +70,7 @@ func typecheckinl(fn *Node) { } if Debug['m'] > 2 || Debug_export != 0 { - fmt.Printf("typecheck import [%v] %L { %#v }\n", fn.Sym, fn, fn.Func.Inl) + fmt.Printf("typecheck import [%v] %L { %#v }\n", fn.Sym, fn, asNodes(fn.Func.Inl.Body)) } save_safemode := safemode @@ -78,9 +78,16 @@ func typecheckinl(fn *Node) { savefn := Curfn Curfn = fn - typecheckslice(fn.Func.Inl.Slice(), Etop) + typecheckslice(fn.Func.Inl.Body, Etop) Curfn = savefn + // During typechecking, declarations are added to + // Curfn.Func.Dcl. Move them to Inl.Dcl for consistency with + // how local functions behave. (Append because typecheckinl + // may be called multiple times.) + fn.Func.Inl.Dcl = append(fn.Func.Inl.Dcl, fn.Func.Dcl...) + fn.Func.Dcl = nil + safemode = save_safemode lineno = lno @@ -155,26 +162,21 @@ func caninl(fn *Node) { return } - savefn := Curfn - Curfn = fn - - n.Func.Inl.Set(fn.Nbody.Slice()) - fn.Nbody.Set(inlcopylist(n.Func.Inl.Slice())) - inldcl := inlcopylist(n.Name.Defn.Func.Dcl) - n.Func.Inldcl.Set(inldcl) - n.Func.InlCost = maxBudget - visitor.budget + n.Func.Inl = &Inline{ + Cost: maxBudget - visitor.budget, + Dcl: inlcopylist(n.Name.Defn.Func.Dcl), + Body: inlcopylist(fn.Nbody.Slice()), + } // hack, TODO, check for better way to link method nodes back to the thing with the ->inl // this is so export can find the body of a method fn.Type.FuncType().Nname = asTypesNode(n) if Debug['m'] > 1 { - fmt.Printf("%v: can inline %#v as: %#v { %#v }\n", fn.Line(), n, fn.Type, n.Func.Inl) + fmt.Printf("%v: can inline %#v as: %#v { %#v }\n", fn.Line(), n, fn.Type, asNodes(n.Func.Inl.Body)) } else if Debug['m'] != 0 { fmt.Printf("%v: can inline %v\n", fn.Line(), n) } - - Curfn = savefn } // inlFlood marks n's inline body for export and recursively ensures @@ -189,7 +191,7 @@ func inlFlood(n *Node) { if n.Func == nil { Fatalf("inlFlood: missing Func on %v", n) } - if n.Func.Inl.Len() == 0 { + if n.Func.Inl == nil { return } @@ -200,7 +202,7 @@ func inlFlood(n *Node) { typecheckinl(n) - inspectList(n.Func.Inl, func(n *Node) bool { + inspectList(asNodes(n.Func.Inl.Body), func(n *Node) bool { switch n.Op { case ONAME: // Mark any referenced global variables or @@ -259,13 +261,13 @@ func (v *hairyVisitor) visit(n *Node) bool { } } - if fn := n.Left.Func; fn != nil && fn.Inl.Len() != 0 { - v.budget -= fn.InlCost + if fn := n.Left.Func; fn != nil && fn.Inl != nil { + v.budget -= fn.Inl.Cost break } if n.Left.isMethodExpression() { - if d := asNode(n.Left.Sym.Def); d != nil && d.Func.Inl.Len() != 0 { - v.budget -= d.Func.InlCost + if d := asNode(n.Left.Sym.Def); d != nil && d.Func.Inl != nil { + v.budget -= d.Func.Inl.Cost break } } @@ -300,8 +302,8 @@ func (v *hairyVisitor) visit(n *Node) bool { break } } - if inlfn := asNode(t.FuncType().Nname).Func; inlfn.Inl.Len() != 0 { - v.budget -= inlfn.InlCost + if inlfn := asNode(t.FuncType().Nname).Func; inlfn.Inl != nil { + v.budget -= inlfn.Inl.Cost break } if Debug['l'] < 4 { @@ -394,7 +396,7 @@ func inlcopy(n *Node) *Node { m := n.copy() if m.Func != nil { - m.Func.Inl.Set(nil) + Fatalf("unexpected Func: %v", m) } m.Left = inlcopy(n.Left) m.Right = inlcopy(n.Right) @@ -583,7 +585,7 @@ func inlnode(n *Node) *Node { if Debug['m'] > 3 { fmt.Printf("%v:call to func %+v\n", n.Line(), n.Left) } - if n.Left.Func != nil && n.Left.Func.Inl.Len() != 0 && !isIntrinsicCall(n) { // normal case + if n.Left.Func != nil && n.Left.Func.Inl != nil && !isIntrinsicCall(n) { // normal case n = mkinlcall(n, n.Left) } else if n.Left.isMethodExpression() && asNode(n.Left.Sym.Def) != nil { n = mkinlcall(n, asNode(n.Left.Sym.Def)) @@ -647,7 +649,7 @@ func inlinableClosure(n *Node) *Node { c := n.Func.Closure caninl(c) f := c.Func.Nname - if f == nil || f.Func.Inl.Len() == 0 { + if f == nil || f.Func.Inl == nil { return nil } return f @@ -772,7 +774,7 @@ var inlgen int // The result of mkinlcall1 MUST be assigned back to n, e.g. // n.Left = mkinlcall1(n.Left, fn, isddd) func mkinlcall1(n, fn *Node) *Node { - if fn.Func.Inl.Len() == 0 { + if fn.Func.Inl == nil { // No inlinable body. return n } @@ -798,7 +800,7 @@ func mkinlcall1(n, fn *Node) *Node { // We have a function node, and it has an inlineable body. if Debug['m'] > 1 { - fmt.Printf("%v: inlining call to %v %#v { %#v }\n", n.Line(), fn.Sym, fn.Type, fn.Func.Inl) + fmt.Printf("%v: inlining call to %v %#v { %#v }\n", n.Line(), fn.Sym, fn.Type, asNodes(fn.Func.Inl.Body)) } else if Debug['m'] != 0 { fmt.Printf("%v: inlining call to %v\n", n.Line(), fn) } @@ -814,12 +816,8 @@ func mkinlcall1(n, fn *Node) *Node { // record formals/locals for later post-processing var inlfvars []*Node - // Find declarations corresponding to inlineable body. - var dcl []*Node + // Handle captured variables when inlining closures. if fn.Name.Defn != nil { - dcl = fn.Func.Inldcl.Slice() // local function - - // handle captured variables when inlining closures if c := fn.Name.Defn.Func.Closure; c != nil { for _, v := range c.Func.Closure.Func.Cvars.Slice() { if v.Op == OXXX { @@ -854,11 +852,9 @@ func mkinlcall1(n, fn *Node) *Node { } } } - } else { - dcl = fn.Func.Dcl // imported function } - for _, ln := range dcl { + for _, ln := range fn.Func.Inl.Dcl { if ln.Op != ONAME { continue } @@ -1020,7 +1016,7 @@ func mkinlcall1(n, fn *Node) *Node { newInlIndex: newIndex, } - body := subst.list(fn.Func.Inl) + body := subst.list(asNodes(fn.Func.Inl.Body)) lab := nod(OLABEL, retlabel, nil) body = append(body, lab) diff --git a/src/cmd/compile/internal/gc/main.go b/src/cmd/compile/internal/gc/main.go index b42966229d..6dd33a2944 100644 --- a/src/cmd/compile/internal/gc/main.go +++ b/src/cmd/compile/internal/gc/main.go @@ -544,7 +544,7 @@ func Main(archInit func(*Arch)) { // Typecheck imported function bodies if debug['l'] > 1, // otherwise lazily when used or re-exported. for _, n := range importlist { - if n.Func.Inl.Len() != 0 { + if n.Func.Inl != nil { saveerrors() typecheckinl(n) } diff --git a/src/cmd/compile/internal/gc/pgen.go b/src/cmd/compile/internal/gc/pgen.go index e9271149a1..9747a0299e 100644 --- a/src/cmd/compile/internal/gc/pgen.go +++ b/src/cmd/compile/internal/gc/pgen.go @@ -570,13 +570,8 @@ func createDwarfVars(fnsym *obj.LSym, fn *Func, automDecls []*Node) ([]*Node, [] // with local vars; disregard this versioning when sorting. func preInliningDcls(fnsym *obj.LSym) []*Node { fn := Ctxt.DwFixups.GetPrecursorFunc(fnsym).(*Node) - var dcl, rdcl []*Node - if fn.Name.Defn != nil { - dcl = fn.Func.Inldcl.Slice() // local function - } else { - dcl = fn.Func.Dcl // imported function - } - for _, n := range dcl { + var rdcl []*Node + for _, n := range fn.Func.Inl.Dcl { c := n.Sym.Name[0] // Avoid reporting "_" parameters, since if there are more than // one, it can result in a collision later on, as in #23179. diff --git a/src/cmd/compile/internal/gc/sizeof_test.go b/src/cmd/compile/internal/gc/sizeof_test.go index c7104030bd..b1184ffbb9 100644 --- a/src/cmd/compile/internal/gc/sizeof_test.go +++ b/src/cmd/compile/internal/gc/sizeof_test.go @@ -22,7 +22,7 @@ func TestSizeof(t *testing.T) { _32bit uintptr // size on 32bit platforms _64bit uintptr // size on 64bit platforms }{ - {Func{}, 124, 224}, + {Func{}, 116, 208}, {Name{}, 32, 56}, {Param{}, 24, 48}, {Node{}, 76, 128}, diff --git a/src/cmd/compile/internal/gc/syntax.go b/src/cmd/compile/internal/gc/syntax.go index 28befbeb85..1b856b1518 100644 --- a/src/cmd/compile/internal/gc/syntax.go +++ b/src/cmd/compile/internal/gc/syntax.go @@ -461,7 +461,6 @@ type Func struct { Exit Nodes Cvars Nodes // closure params Dcl []*Node // autodcl for this func/closure - Inldcl Nodes // copy of dcl for use in inlining // Parents records the parent scope of each scope within a // function. The root scope (0) has no parent, so the i'th @@ -484,8 +483,7 @@ type Func struct { Nname *Node lsym *obj.LSym - Inl Nodes // copy of the body for use in inlining - InlCost int32 + Inl *Inline Label int32 // largest auto-generated label in this function @@ -502,6 +500,15 @@ type Func struct { nwbrCalls *[]nowritebarrierrecCallSym } +// An Inline holds fields used for function bodies that can be inlined. +type Inline struct { + Cost int32 // heuristic cost of inlining this function + + // Copies of Func.Dcl and Nbody for use during inlining. + Dcl []*Node + Body []*Node +} + // A Mark represents a scope boundary. type Mark struct { // Pos is the position of the token that marks the scope @@ -737,6 +744,11 @@ const ( // a slice to save space. type Nodes struct{ slice *[]*Node } +// asNodes returns a slice of *Node as a Nodes value. +func asNodes(s []*Node) Nodes { + return Nodes{&s} +} + // Slice returns the entries in Nodes as a slice. // Changes to the slice entries (as in s[i] = n) will be reflected in // the Nodes. -- 2.50.0