Skip to content

Commit 3d74134

Browse files
committed
cmd/compile: collect reasons in inlining test
If we use -gcflags='-m -m', the compiler should give us a reason why a func couldn't be inlined. Add the extra -m necessary for that extra info and use it to give better test failures. For example, for the func in the TODO: --- FAIL: TestIntendedInlining (1.53s) inl_test.go:104: runtime.nextFreeFast was not inlined: function too complex We might increase the number of -m flags to get more information at some later point, such as getting details on how close the func was to the inlining budget. Also started using regexes, as the output parsing is getting a bit too complex for manual string handling. While at it, also refactored the test to not buffer the entire output into memory. This is fine in practice, but it won't scale well as we add more packages or we depend more on the compiler's debugging output. For example, "go build -a -gcflags='-m -m' std" prints nearly 40MB of plaintext - and we only need to see the output line by line anyway. Updates #21851. Change-Id: I00986ff360eb56e4e9737b65a6be749ef8540643 Reviewed-on: https://go-review.googlesource.com/63810 Run-TryBot: Daniel Martí <[email protected]> TryBot-Result: Gobot Gobot <[email protected]> Reviewed-by: Matthew Dempsky <[email protected]>
1 parent 7a5d76f commit 3d74134

File tree

1 file changed

+43
-20
lines changed

1 file changed

+43
-20
lines changed

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

+43-20
Original file line numberDiff line numberDiff line change
@@ -5,10 +5,13 @@
55
package gc
66

77
import (
8-
"bytes"
8+
"bufio"
99
"internal/testenv"
10+
"io"
1011
"os/exec"
12+
"regexp"
1113
"runtime"
14+
"strings"
1215
"testing"
1316
)
1417

@@ -51,37 +54,57 @@ func TestIntendedInlining(t *testing.T) {
5154
want["runtime"] = append(want["runtime"], "nextFreeFast")
5255
}
5356

54-
m := make(map[string]bool)
57+
notInlinedReason := make(map[string]string)
5558
pkgs := make([]string, 0, len(want))
5659
for pname, fnames := range want {
5760
pkgs = append(pkgs, pname)
5861
for _, fname := range fnames {
59-
m[pname+"."+fname] = true
62+
notInlinedReason[pname+"."+fname] = "unknown reason"
6063
}
6164
}
6265

63-
args := append([]string{"build", "-a", "-gcflags=-m"}, pkgs...)
66+
args := append([]string{"build", "-a", "-gcflags=-m -m"}, pkgs...)
6467
cmd := testenv.CleanCmdEnv(exec.Command(testenv.GoToolPath(t), args...))
65-
out, err := cmd.CombinedOutput()
66-
if err != nil {
67-
t.Logf("%s", out)
68-
t.Fatal(err)
69-
}
70-
lines := bytes.Split(out, []byte{'\n'})
68+
pr, pw := io.Pipe()
69+
cmd.Stdout = pw
70+
cmd.Stderr = pw
71+
cmdErr := make(chan error, 1)
72+
go func() {
73+
cmdErr <- cmd.Run()
74+
pw.Close()
75+
}()
76+
scanner := bufio.NewScanner(pr)
7177
curPkg := ""
72-
for _, l := range lines {
73-
if bytes.HasPrefix(l, []byte("# ")) {
74-
curPkg = string(l[2:])
78+
canInline := regexp.MustCompile(`: can inline ([^ ]*)`)
79+
cannotInline := regexp.MustCompile(`: cannot inline ([^ ]*): (.*)`)
80+
for scanner.Scan() {
81+
line := scanner.Text()
82+
if strings.HasPrefix(line, "# ") {
83+
curPkg = line[2:]
84+
continue
85+
}
86+
if m := canInline.FindStringSubmatch(line); m != nil {
87+
fname := m[1]
88+
delete(notInlinedReason, curPkg+"."+fname)
89+
continue
7590
}
76-
f := bytes.Split(l, []byte(": can inline "))
77-
if len(f) < 2 {
91+
if m := cannotInline.FindStringSubmatch(line); m != nil {
92+
fname, reason := m[1], m[2]
93+
fullName := curPkg + "." + fname
94+
if _, ok := notInlinedReason[fullName]; ok {
95+
// cmd/compile gave us a reason why
96+
notInlinedReason[fullName] = reason
97+
}
7898
continue
7999
}
80-
fn := bytes.TrimSpace(f[1])
81-
delete(m, curPkg+"."+string(fn))
82100
}
83-
84-
for s := range m {
85-
t.Errorf("function %s not inlined", s)
101+
if err := <-cmdErr; err != nil {
102+
t.Fatal(err)
103+
}
104+
if err := scanner.Err(); err != nil {
105+
t.Fatal(err)
106+
}
107+
for fullName, reason := range notInlinedReason {
108+
t.Errorf("%s was not inlined: %s", fullName, reason)
86109
}
87110
}

0 commit comments

Comments
 (0)