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

This reverts commit http://go.dev/cl/c/482356.

Reason for revert: Reverting this change again, since it is causing additional failures in google-internal testing.

Change-Id: I9234946f62e5bb18c2f873a65e8b298d04af0809
Reviewed-on: https://go-review.googlesource.com/c/go/+/484735
Reviewed-by: Florian Zenker <[email protected]>
Run-TryBot: Than McIntosh <[email protected]>
Auto-Submit: Than McIntosh <[email protected]>
Reviewed-by: Than McIntosh <[email protected]>
TryBot-Result: Gopher Robot <[email protected]>
Reviewed-by: Cuong Manh Le <[email protected]>

Author: thanm

src/cmd/compile/internal/inline/inl.go

@@ -459,8 +459,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
@@ -592,10 +590,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,

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",

test/closure3.dir/main.go

@@ -232,53 +232,49 @@ 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")
 		}
 	}
 
 	{
 		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 {