Skip to content

Commit 170436c

Browse files
committed
cmd/compile: strongly favor closure inlining
This tweaks the inlining cost knob for closures specifically, they receive a doubled budget. The rationale for this is that closures have a lot of "crud" in their IR that will disappear after inlining, so the standard budget penalizes them unnecessarily. This is also the cause of these bugs -- looking at the code involved, these closures "should" be inlineable, therefore tweak the parameters until behavior matches expectations. It's not costly in binary size, because the only-called-from-one-site case is common (especially for rangefunc iterators). I can imagine better fixes and I am going to try to get that done, but this one is small and makes things better. Fixes #69411, #69539. Change-Id: I8a892c40323173a723799e0ddad69dcc2724a8f9 Reviewed-on: https://go-review.googlesource.com/c/go/+/629195 Reviewed-by: Keith Randall <khr@google.com> Reviewed-by: Keith Randall <khr@golang.org> Reviewed-by: Robert Griesemer <gri@google.com> LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
1 parent b8fe883 commit 170436c

File tree

3 files changed

+19
-11
lines changed

3 files changed

+19
-11
lines changed

src/cmd/cgo/internal/test/callback_windows.go

+3-2
Original file line numberDiff line numberDiff line change
@@ -69,11 +69,11 @@ func testCallbackCallersSEH(t *testing.T) {
6969
want := []string{
7070
"test._Cfunc_backtrace",
7171
"test.testCallbackCallersSEH.func1.1",
72-
"test.testCallbackCallersSEH.func1",
72+
// "test.testCallbackCallersSEH.func1", // hidden by inlining
7373
"test.goCallback",
7474
"test._Cfunc_callback",
7575
"test.nestedCall.func1",
76-
"test.nestedCall",
76+
// "test.nestedCall", // hidden by inlining
7777
"test.testCallbackCallersSEH",
7878
"test.TestCallbackCallersSEH",
7979
}
@@ -84,6 +84,7 @@ func testCallbackCallersSEH(t *testing.T) {
8484
})
8585
got := make([]string, 0, n)
8686
for i := 0; i < n; i++ {
87+
// This test is brittle in the face of inliner changes
8788
f := runtime.FuncForPC(pc[i] - 1)
8889
if f == nil {
8990
continue

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

+7
Original file line numberDiff line numberDiff line change
@@ -206,6 +206,9 @@ func inlineBudget(fn *ir.Func, profile *pgoir.Profile, relaxed bool, verbose boo
206206
if relaxed {
207207
budget += inlheur.BudgetExpansion(inlineMaxBudget)
208208
}
209+
if fn.ClosureParent != nil {
210+
budget *= 2
211+
}
209212
return budget
210213
}
211214

@@ -861,6 +864,10 @@ var InlineCall = func(callerfn *ir.Func, call *ir.CallExpr, fn *ir.Func, inlInde
861864
// - whether the inlined function is "hot" according to PGO.
862865
func inlineCostOK(n *ir.CallExpr, caller, callee *ir.Func, bigCaller bool) (bool, int32, int32, bool) {
863866
maxCost := int32(inlineMaxBudget)
867+
if callee.ClosureParent != nil {
868+
maxCost *= 2 // favor inlining closures
869+
}
870+
864871
if bigCaller {
865872
// We use this to restrict inlining into very big functions.
866873
// See issue 26546 and 17566.

test/closure3.dir/main.go

+9-9
Original file line numberDiff line numberDiff line change
@@ -221,14 +221,14 @@ func main() {
221221

222222
{
223223
b := 2
224-
func(b int) { // ERROR "func literal does not escape"
225-
func() { // ERROR "can inline main.func25.1"
224+
func(b int) { // ERROR "can inline main.func25"
225+
func() { // ERROR "can inline main.func25.1" "can inline main.main.func25.func33"
226226
b = 3
227227
}() // ERROR "inlining call to main.func25.1"
228228
if b != 3 {
229229
ppanic("b != 3")
230230
}
231-
}(b)
231+
}(b) // ERROR "inlining call to main.func25" "inlining call to main.main.func25.func33"
232232
if b != 2 {
233233
ppanic("b != 2")
234234
}
@@ -258,13 +258,13 @@ func main() {
258258
// revisit those. E.g., func34 and func36 are constructed by the inliner.
259259
if r := func(x int) int { // ERROR "can inline main.func27"
260260
b := 3
261-
return func(y int) int { // ERROR "can inline main.func27.1" "can inline main.main.func27.func34"
261+
return func(y int) int { // ERROR "can inline main.func27.1" "can inline main.main.func27.func35"
262262
c := 5
263-
return func(z int) int { // ERROR "can inline main.func27.1.1" "can inline main.main.func27.func34.1" "can inline main.func27.main.func27.1.2" "can inline main.main.func27.main.main.func27.func34.func36"
263+
return func(z int) int { // ERROR "can inline main.func27.1.1" "can inline main.main.func27.func35.1" "can inline main.func27.main.func27.1.2" "can inline main.main.func27.main.main.func27.func35.func37"
264264
return a*x + b*y + c*z
265265
}(10) // ERROR "inlining call to main.func27.1.1"
266266
}(100) // ERROR "inlining call to main.func27.1" "inlining call to main.func27.main.func27.1.2"
267-
}(1000); r != 2350 { // ERROR "inlining call to main.func27" "inlining call to main.main.func27.func34" "inlining call to main.main.func27.main.main.func27.func34.func36"
267+
}(1000); r != 2350 { // ERROR "inlining call to main.func27" "inlining call to main.main.func27.func35" "inlining call to main.main.func27.main.main.func27.func35.func37"
268268
ppanic("r != 2350")
269269
}
270270
}
@@ -273,16 +273,16 @@ func main() {
273273
a := 2
274274
if r := func(x int) int { // ERROR "can inline main.func28"
275275
b := 3
276-
return func(y int) int { // ERROR "can inline main.func28.1" "can inline main.main.func28.func35"
276+
return func(y int) int { // ERROR "can inline main.func28.1" "can inline main.main.func28.func36"
277277
c := 5
278-
func(z int) { // ERROR "can inline main.func28.1.1" "can inline main.func28.main.func28.1.2" "can inline main.main.func28.func35.1" "can inline main.main.func28.main.main.func28.func35.func37"
278+
func(z int) { // ERROR "can inline main.func28.1.1" "can inline main.func28.main.func28.1.2" "can inline main.main.func28.func36.1" "can inline main.main.func28.main.main.func28.func36.func38"
279279
a = a * x
280280
b = b * y
281281
c = c * z
282282
}(10) // ERROR "inlining call to main.func28.1.1"
283283
return a + c
284284
}(100) + b // ERROR "inlining call to main.func28.1" "inlining call to main.func28.main.func28.1.2"
285-
}(1000); r != 2350 { // ERROR "inlining call to main.func28" "inlining call to main.main.func28.func35" "inlining call to main.main.func28.main.main.func28.func35.func37"
285+
}(1000); r != 2350 { // ERROR "inlining call to main.func28" "inlining call to main.main.func28.func36" "inlining call to main.main.func28.main.main.func28.func36.func38"
286286
ppanic("r != 2350")
287287
}
288288
if a != 2000 {

0 commit comments

Comments
 (0)