Skip to content

Commit

Permalink
Revert "cmd/compile: allow more inlining of functions that construct …
Browse files Browse the repository at this point in the history
…closures"

This reverts commit f8162a0.

Reason for revert: #59680

Change-Id: I91821c691a2d019ff0ad5b69509e32f3d56b8f67
Reviewed-on: https://go-review.googlesource.com/c/go/+/485498
Reviewed-by: Russ Cox <[email protected]>
TryBot-Result: Gopher Robot <[email protected]>
Run-TryBot: Michael Knyszek <[email protected]>
Reviewed-by: Matthew Dempsky <[email protected]>
Auto-Submit: Michael Knyszek <[email protected]>
  • Loading branch information
mknyszek authored and gopherbot committed Apr 17, 2023
1 parent 94850c6 commit ce10e9d
Show file tree
Hide file tree
Showing 3 changed files with 30 additions and 29 deletions.
11 changes: 6 additions & 5 deletions src/cmd/compile/internal/inline/inl.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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,
Expand Down
22 changes: 13 additions & 9 deletions src/cmd/compile/internal/test/inl_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
26 changes: 11 additions & 15 deletions test/closure3.dir/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down

0 comments on commit ce10e9d

Please sign in to comment.