Skip to content

Commit a509cae

Browse files
committed
cmd/compile: record InlCost in export data
Previously, we were treating cross-package function calls as free for inlining budgeting. In theory, we should be able to recompute InlCost from the exported/reimported function bodies. However, that process mutates the structure of the Node AST enough that it doesn't preserve InlCost. To avoid unexpected issues, just record and restore InlCost in the export data. Fixes #19261. Change-Id: Iac2bc0d32d4f948b64524aca657051f9fc96d92d Reviewed-on: https://go-review.googlesource.com/70151 Run-TryBot: Matthew Dempsky <[email protected]> Reviewed-by: Robert Griesemer <[email protected]>
1 parent 1fbeccb commit a509cae

File tree

5 files changed

+51
-0
lines changed

5 files changed

+51
-0
lines changed

src/cmd/compile/internal/gc/bexport.go

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -377,6 +377,7 @@ func export(out *bufio.Writer, trace bool) int {
377377
p.tracef("\n----\nfunc { %#v }\n", f.Inl)
378378
}
379379
p.int(i)
380+
p.int(int(f.InlCost))
380381
p.stmtList(f.Inl)
381382
if p.trace {
382383
p.tracef("\n")

src/cmd/compile/internal/gc/bimport.go

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -187,6 +187,7 @@ func Import(imp *types.Pkg, in *bufio.Reader) {
187187
// them only for functions with inlineable bodies. funchdr does
188188
// parameter renaming which doesn't matter if we don't have a body.
189189

190+
inlCost := p.int()
190191
if f := p.funcList[i]; f != nil {
191192
// function not yet imported - read body and set it
192193
funchdr(f)
@@ -200,6 +201,7 @@ func Import(imp *types.Pkg, in *bufio.Reader) {
200201
body = []*Node{nod(OEMPTY, nil, nil)}
201202
}
202203
f.Func.Inl.Set(body)
204+
f.Func.InlCost = int32(inlCost)
203205
funcbody()
204206
} else {
205207
// function already imported - read body but discard declarations

test/fixedbugs/issue19261.dir/p.go

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,24 @@
1+
// Copyright 2017 The Go Authors. All rights reserved.
2+
// Use of this source code is governed by a BSD-style
3+
// license that can be found in the LICENSE file.
4+
5+
package p
6+
7+
func F() { // ERROR "can inline F"
8+
print(1, 2, 3, 4, 5, 6, 7, 8, 9, 10)
9+
print(1, 2, 3, 4, 5, 6, 7, 8, 9, 10)
10+
print(1, 2, 3, 4, 5, 6, 7, 8, 9, 10)
11+
print(1, 2, 3, 4, 5, 6, 7, 8, 9, 10)
12+
print(1, 2, 3, 4, 5, 6, 7, 8, 9, 10)
13+
print(1, 2, 3, 4, 5, 6, 7, 8, 9, 10)
14+
}
15+
16+
func G() {
17+
F() // ERROR "inlining call to F"
18+
print(1, 2, 3, 4, 5, 6, 7, 8, 9, 10)
19+
print(1, 2, 3, 4, 5, 6, 7, 8, 9, 10)
20+
print(1, 2, 3, 4, 5, 6, 7, 8, 9, 10)
21+
print(1, 2, 3, 4, 5, 6, 7, 8, 9, 10)
22+
print(1, 2, 3, 4, 5, 6, 7, 8, 9, 10)
23+
print(1, 2, 3, 4, 5, 6, 7, 8, 9, 10)
24+
}

test/fixedbugs/issue19261.dir/q.go

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,17 @@
1+
// Copyright 2017 The Go Authors. All rights reserved.
2+
// Use of this source code is governed by a BSD-style
3+
// license that can be found in the LICENSE file.
4+
5+
package q
6+
7+
import "./p"
8+
9+
func H() {
10+
p.F() // ERROR "inlining call to p.F"
11+
print(1, 2, 3, 4, 5, 6, 7, 8, 9, 10)
12+
print(1, 2, 3, 4, 5, 6, 7, 8, 9, 10)
13+
print(1, 2, 3, 4, 5, 6, 7, 8, 9, 10)
14+
print(1, 2, 3, 4, 5, 6, 7, 8, 9, 10)
15+
print(1, 2, 3, 4, 5, 6, 7, 8, 9, 10)
16+
print(1, 2, 3, 4, 5, 6, 7, 8, 9, 10)
17+
}

test/fixedbugs/issue19261.go

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
// errorcheckdir -0 -m
2+
3+
// Copyright 2017 The Go Authors. All rights reserved.
4+
// Use of this source code is governed by a BSD-style
5+
// license that can be found in the LICENSE file.
6+
7+
package ignored

0 commit comments

Comments
 (0)