From 1bcf6beec53ae811490fcd0ac29328b12b53702c Mon Sep 17 00:00:00 2001 From: Matthew Dempsky Date: Tue, 22 Sep 2020 02:19:14 -0700 Subject: [PATCH] cmd/compile: use staticValue for inlining logic This CL replaces the ad hoc and duplicated logic for detecting inlinable calls with a single "inlCallee" function, which uses the "staticValue" helper function introduced in an earlier commit. Updates #41474. Change-Id: I103d4091b10366fce1344ef2501222b7df68f21d Reviewed-on: https://go-review.googlesource.com/c/go/+/256460 Reviewed-by: David Chase Reviewed-by: Cuong Manh Le Trust: Matthew Dempsky --- src/cmd/compile/internal/gc/inl.go | 92 +++++-------------- .../compile/internal/logopt/logopt_test.go | 1 - test/inline.go | 9 ++ 3 files changed, 34 insertions(+), 68 deletions(-) diff --git a/src/cmd/compile/internal/gc/inl.go b/src/cmd/compile/internal/gc/inl.go index 8630560a9a..ba12cf40b5 100644 --- a/src/cmd/compile/internal/gc/inl.go +++ b/src/cmd/compile/internal/gc/inl.go @@ -325,18 +325,10 @@ func (v *hairyVisitor) visit(n *Node) bool { break } - if fn := n.Left.Func; fn != nil && fn.Inl != nil { - v.budget -= fn.Inl.Cost + if fn := inlCallee(n.Left); fn != nil && fn.Func.Inl != nil { + v.budget -= fn.Func.Inl.Cost break } - if n.Left.isMethodExpression() { - if d := asNode(n.Left.Sym.Def); d != nil && d.Func.Inl != nil { - v.budget -= d.Func.Inl.Cost - break - } - } - // TODO(mdempsky): Budget for OCLOSURE calls if we - // ever allow that. See #15561 and #23093. // Call cost for non-leaf inlining. v.budget -= v.extraCallCost @@ -679,53 +671,11 @@ func inlnode(n *Node, maxCost int32, inlMap map[*Node]bool) *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 != nil && !isIntrinsicCall(n) { // normal case - n = mkinlcall(n, n.Left, maxCost, inlMap) - } else if n.Left.isMethodExpression() && asNode(n.Left.Sym.Def) != nil { - n = mkinlcall(n, asNode(n.Left.Sym.Def), maxCost, inlMap) - } else if n.Left.Op == OCLOSURE { - if f := inlinableClosure(n.Left); f != nil { - n = mkinlcall(n, f, maxCost, inlMap) - } - } else if n.Left.Op == ONAME && n.Left.Name != nil && n.Left.Name.Defn != nil { - if d := n.Left.Name.Defn; d.Op == OAS && d.Right.Op == OCLOSURE { - if f := inlinableClosure(d.Right); f != nil { - // NB: this check is necessary to prevent indirect re-assignment of the variable - // having the address taken after the invocation or only used for reads is actually fine - // but we have no easy way to distinguish the safe cases - if d.Left.Name.Addrtaken() { - if Debug['m'] > 1 { - fmt.Printf("%v: cannot inline escaping closure variable %v\n", n.Line(), n.Left) - } - if logopt.Enabled() { - logopt.LogOpt(n.Pos, "cannotInlineCall", "inline", Curfn.funcname(), - fmt.Sprintf("%v cannot be inlined (escaping closure variable)", n.Left)) - } - break - } - - // ensure the variable is never re-assigned - if unsafe, a := reassigned(n.Left); unsafe { - if Debug['m'] > 1 { - if a != nil { - fmt.Printf("%v: cannot inline re-assigned closure variable at %v: %v\n", n.Line(), a.Line(), a) - if logopt.Enabled() { - logopt.LogOpt(n.Pos, "cannotInlineCall", "inline", Curfn.funcname(), - fmt.Sprintf("%v cannot be inlined (re-assigned closure variable)", a)) - } - } else { - fmt.Printf("%v: cannot inline global closure variable %v\n", n.Line(), n.Left) - if logopt.Enabled() { - logopt.LogOpt(n.Pos, "cannotInlineCall", "inline", Curfn.funcname(), - fmt.Sprintf("%v cannot be inlined (global closure variable)", n.Left)) - } - } - } - break - } - n = mkinlcall(n, f, maxCost, inlMap) - } - } + if isIntrinsicCall(n) { + break + } + if fn := inlCallee(n.Left); fn != nil && fn.Func.Inl != nil { + n = mkinlcall(n, fn, maxCost, inlMap) } case OCALLMETH: @@ -749,16 +699,22 @@ func inlnode(n *Node, maxCost int32, inlMap map[*Node]bool) *Node { return n } -// inlinableClosure takes an OCLOSURE node and follows linkage to the matching ONAME with -// the inlinable body. Returns nil if the function is not inlinable. -func inlinableClosure(n *Node) *Node { - c := n.Func.Closure - caninl(c) - f := c.Func.Nname - if f == nil || f.Func.Inl == nil { - return nil +// inlCallee takes a function-typed expression and returns the underlying function ONAME +// that it refers to if statically known. Otherwise, it returns nil. +func inlCallee(fn *Node) *Node { + fn = staticValue(fn) + switch { + case fn.Op == ONAME && fn.Class() == PFUNC: + if fn.isMethodExpression() { + return asNode(fn.Sym.Def) + } + return fn + case fn.Op == OCLOSURE: + c := fn.Func.Closure + caninl(c) + return c.Func.Nname } - return f + return nil } func staticValue(n *Node) *Node { @@ -771,7 +727,9 @@ func staticValue(n *Node) *Node { } } -// staticValue1 implements a simple SSA-like optimization. +// staticValue1 implements a simple SSA-like optimization. If n is a local variable +// that is initialized and never reassigned, staticValue1 returns the initializer +// expression. Otherwise, it returns nil. func staticValue1(n *Node) *Node { if n.Op != ONAME || n.Class() != PAUTO || n.Name.Addrtaken() { return nil diff --git a/src/cmd/compile/internal/logopt/logopt_test.go b/src/cmd/compile/internal/logopt/logopt_test.go index fb71e142e3..fca85c10fb 100644 --- a/src/cmd/compile/internal/logopt/logopt_test.go +++ b/src/cmd/compile/internal/logopt/logopt_test.go @@ -208,7 +208,6 @@ func s15a8(x *[15]int64) [15]int64 { `"relatedInformation":[{"location":{"uri":"file://tmpdir/file.go","range":{"start":{"line":4,"character":11},"end":{"line":4,"character":11}}},"message":"inlineLoc"}]}`) want(t, slogged, `{"range":{"start":{"line":11,"character":6},"end":{"line":11,"character":6}},"severity":3,"code":"isInBounds","source":"go compiler","message":""}`) want(t, slogged, `{"range":{"start":{"line":7,"character":6},"end":{"line":7,"character":6}},"severity":3,"code":"canInlineFunction","source":"go compiler","message":"cost: 35"}`) - want(t, slogged, `{"range":{"start":{"line":21,"character":21},"end":{"line":21,"character":21}},"severity":3,"code":"cannotInlineCall","source":"go compiler","message":"foo cannot be inlined (escaping closure variable)"}`) // escape analysis explanation want(t, slogged, `{"range":{"start":{"line":7,"character":13},"end":{"line":7,"character":13}},"severity":3,"code":"leak","source":"go compiler","message":"parameter z leaks to ~r2 with derefs=0",`+ `"relatedInformation":[`+ diff --git a/test/inline.go b/test/inline.go index 2f6fc0fe88..0e41873de4 100644 --- a/test/inline.go +++ b/test/inline.go @@ -49,6 +49,12 @@ func j(x int) int { // ERROR "can inline j" } } +func _() int { // ERROR "can inline _" + tmp1 := h + tmp2 := tmp1 + return tmp2(0) // ERROR "inlining call to h" +} + var somethingWrong error // local closures can be inlined @@ -58,6 +64,9 @@ func l(x, y int) (int, int, error) { } if x == y { e(somethingWrong) // ERROR "inlining call to l.func1" + } else { + f := e + f(nil) // ERROR "inlining call to l.func1" } return y, x, nil } -- 2.48.1