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 <drchase@google.com>
Reviewed-by: Cuong Manh Le <cuong.manhle.vn@gmail.com>
Trust: Matthew Dempsky <mdempsky@google.com>

diff --git a/src/cmd/compile/internal/gc/inl.go b/src/cmd/compile/internal/gc/inl.go
index 8630560a9afaa04f0636acac145e152396859f82..ba12cf40b51ba7d4fc1ab5e2d743b09f96c20648 100644 (file)
--- 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 fb71e142e3c58f69a57ac551e86aed5a1d5d3217..fca85c10fbb38539f6ee0812ec228122d35720a5 100644 (file)
--- 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 2f6fc0fe88066a86a05f475b7a3353d182704e0b..0e41873de4ba5e3fed3ea2d2ac0d144f61a12a6e 100644 (file)
--- 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
 }