Revert "cmd/compile: allow more inlining of functions that construct closures"

This reverts commit f8162a0e726f4b3a9df60a37e8ca7883dde61914.

Reason for revert: https://github.com/golang/go/issues/59680

Change-Id: I91821c691a2d019ff0ad5b69509e32f3d56b8f67
Reviewed-on: https://go-review.googlesource.com/c/go/+/485498
Reviewed-by: Russ Cox <rsc@golang.org>
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Michael Knyszek <mknyszek@google.com>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
Auto-Submit: Michael Knyszek <mknyszek@google.com>

diff --git a/src/cmd/compile/internal/inline/inl.go b/src/cmd/compile/internal/inline/inl.go
index 81aa200a84607f1255cbedeb3a594736d54718a8..24812c8c0d67f715de07f36e02bc12a4f63c68fe 100644 (file)
--- a/src/cmd/compile/internal/inline/inl.go
+++ b/src/cmd/compile/internal/inline/inl.go
@@ -506,8 +506,6 @@ func (v *hairyVisitor) tooHairy(fn *ir.Func) bool {
        return false
 }
 
-// doNode visits n and its children, updates the state in v, and returns true if
-// n makes the current function too hairy for inlining.
 func (v *hairyVisitor) doNode(n ir.Node) bool {
        if n == nil {
                return false
@@ -639,10 +637,13 @@ func (v *hairyVisitor) doNode(n ir.Node) bool {
                // TODO(danscales): Maybe make budget proportional to number of closure
                // variables, e.g.:
                //v.budget -= int32(len(n.(*ir.ClosureExpr).Func.ClosureVars) * 3)
-               // TODO(austin): However, if we're able to inline this closure into
-               // v.curFunc, then we actually pay nothing for the closure captures. We
-               // should try to account for that if we're going to account for captures.
                v.budget -= 15
+               // Scan body of closure (which DoChildren doesn't automatically
+               // do) to check for disallowed ops in the body and include the
+               // body in the budget.
+               if doList(n.(*ir.ClosureExpr).Func.Body, v.do) {
+                       return true
+               }
 
        case ir.OGO,
                ir.ODEFER,

diff --git a/src/cmd/compile/internal/test/inl_test.go b/src/cmd/compile/internal/test/inl_test.go
index 205b746dd8925f31c4ca0a9f56e597949793bcc9..2a16b21cef76ea77dacd493da4ed6da86ab0cca9 100644 (file)
--- a/src/cmd/compile/internal/test/inl_test.go
+++ b/src/cmd/compile/internal/test/inl_test.go
@@ -180,15 +180,19 @@ func TestIntendedInlining(t *testing.T) {
                "net": {
                        "(*UDPConn).ReadFromUDP",
                },
-               "sync": {
-                       // Both OnceFunc and its returned closure need to be inlinable so
-                       // that the returned closure can be inlined into the caller of OnceFunc.
-                       "OnceFunc",
-                       "OnceFunc.func2", // The returned closure.
-                       // TODO(austin): It would be good to check OnceValue and OnceValues,
-                       // too, but currently they aren't reported because they have type
-                       // parameters and aren't instantiated in sync.
-               },
+               // These testpoints commented out for now, since CL 479095
+               // had to be reverted. We can re-enable this once we roll
+               // forward with a new version of 479095.
+               /*
+                       "sync": {
+                               // Both OnceFunc and its returned closure need to be inlinable so
+                               // that the returned closure can be inlined into the caller of OnceFunc.
+                               "OnceFunc",
+                               "OnceFunc.func2", // The returned closure.
+                               // TODO(austin): It would be good to check OnceValue and OnceValues,
+                               // too, but currently they aren't reported because they have type
+                               // parameters and aren't instantiated in sync.
+                       }, */
                "sync/atomic": {
                        // (*Bool).CompareAndSwap handled below.
                        "(*Bool).Load",

diff --git a/test/closure3.dir/main.go b/test/closure3.dir/main.go
index 04a669206e53382256369961807e678616aaee8e..4d02a4d10ede896e89c392e543242ded2756c823 100644 (file)
--- a/test/closure3.dir/main.go
+++ b/test/closure3.dir/main.go
@@ -232,15 +232,15 @@ func main() {
 
        {
                c := 3
-               func() { // ERROR "can inline main.func26"
+               func() { // ERROR "func literal does not escape"
                        c = 4
-                       func() {
+                       func() { // ERROR "func literal does not escape"
                                if c != 4 {
                                        ppanic("c != 4")
                                }
                                recover() // prevent inlining
                        }()
-               }() // ERROR "inlining call to main.func26" "func literal does not escape"
+               }()
                if c != 4 {
                        ppanic("c != 4")
                }
@@ -248,37 +248,33 @@ func main() {
 
        {
                a := 2
-               // This has an unfortunate exponential growth, where as we visit each
-               // function, we inline the inner closure, and that constructs a new
-               // function for any closures inside the inner function, and then we
-               // revisit those. E.g., func34 and func36 are constructed by the inliner.
-               if r := func(x int) int { // ERROR "can inline main.func27"
+               if r := func(x int) int { // ERROR "func literal does not escape"
                        b := 3
-                       return func(y int) int { // ERROR "can inline main.func27.1" "can inline main.func34"
+                       return func(y int) int { // ERROR "can inline main.func27.1"
                                c := 5
-                               return func(z int) int { // ERROR "can inline main.func27.1.1" "can inline main.func27.(func)?2" "can inline main.func34.1" "can inline main.func36"
+                               return func(z int) int { // ERROR "can inline main.func27.1.1" "can inline main.func27.(func)?2"
                                        return a*x + b*y + c*z
                                }(10) // ERROR "inlining call to main.func27.1.1"
                        }(100) // ERROR "inlining call to main.func27.1" "inlining call to main.func27.(func)?2"
-               }(1000); r != 2350 { // ERROR "inlining call to main.func27" "inlining call to main.func34" "inlining call to main.func36"
+               }(1000); r != 2350 {
                        ppanic("r != 2350")
                }
        }
 
        {
                a := 2
-               if r := func(x int) int { // ERROR "can inline main.func28"
+               if r := func(x int) int { // ERROR "func literal does not escape"
                        b := 3
-                       return func(y int) int { // ERROR "can inline main.func28.1" "can inline main.func35"
+                       return func(y int) int { // ERROR "can inline main.func28.1"
                                c := 5
-                               func(z int) { // ERROR "can inline main.func28.1.1" "can inline main.func28.(func)?2" "can inline main.func35.1" "can inline main.func37"
+                               func(z int) { // ERROR "can inline main.func28.1.1" "can inline main.func28.(func)?2"
                                        a = a * x
                                        b = b * y
                                        c = c * z
                                }(10) // ERROR "inlining call to main.func28.1.1"
                                return a + c
                        }(100) + b // ERROR "inlining call to main.func28.1" "inlining call to main.func28.(func)?2"
-               }(1000); r != 2350 { // ERROR "inlining call to main.func28" "inlining call to main.func35" "inlining call to main.func37"
+               }(1000); r != 2350 {
                        ppanic("r != 2350")
                }
                if a != 2000 {