From fced302aa15702056e0d4a264c80e74c462cdd22 Mon Sep 17 00:00:00 2001 From: David Chase Date: Wed, 4 Mar 2020 16:33:54 -0500 Subject: [PATCH] cmd/compile: change gc logging to report inline failure instead of success I've been experimenting with this, success is the wrong thing to report even though it seems to log much less. Change-Id: I7c25a45d2f41e82b6c8dd8b0a56ba848c63fb21a Reviewed-on: https://go-review.googlesource.com/c/go/+/223298 Run-TryBot: David Chase TryBot-Result: Gobot Gobot Reviewed-by: Cherry Zhang --- src/cmd/compile/internal/gc/inl.go | 20 +++++++++++++++---- .../compile/internal/logopt/logopt_test.go | 17 ++++++++++++---- 2 files changed, 29 insertions(+), 8 deletions(-) diff --git a/src/cmd/compile/internal/gc/inl.go b/src/cmd/compile/internal/gc/inl.go index 29210ff8de..272d0bdab7 100644 --- a/src/cmd/compile/internal/gc/inl.go +++ b/src/cmd/compile/internal/gc/inl.go @@ -687,6 +687,10 @@ func inlnode(n *Node, maxCost int32, inlMap map[*Node]bool) *Node { if Debug['m'] > 1 { fmt.Printf("%v: cannot inline escaping closure variable %v\n", n.Line(), n.Left) } + if logopt.Enabled() { + logopt.LogOpt(n.Pos, "cannotInlineCall", "inline", Curfn.funcname(), + fmt.Sprintf("%v cannot be inlined (escaping closure variable)", n.Left)) + } break } @@ -695,8 +699,16 @@ func inlnode(n *Node, maxCost int32, inlMap map[*Node]bool) *Node { if Debug['m'] > 1 { if a != nil { fmt.Printf("%v: cannot inline re-assigned closure variable at %v: %v\n", n.Line(), a.Line(), a) + if logopt.Enabled() { + logopt.LogOpt(n.Pos, "cannotInlineCall", "inline", Curfn.funcname(), + fmt.Sprintf("%v cannot be inlined (re-assigned closure variable)", a)) + } } else { fmt.Printf("%v: cannot inline global closure variable %v\n", n.Line(), n.Left) + if logopt.Enabled() { + logopt.LogOpt(n.Pos, "cannotInlineCall", "inline", Curfn.funcname(), + fmt.Sprintf("%v cannot be inlined (global closure variable)", n.Left)) + } } } break @@ -842,7 +854,10 @@ var inlgen int // n.Left = mkinlcall(n.Left, fn, isddd) func mkinlcall(n, fn *Node, maxCost int32, inlMap map[*Node]bool) *Node { if fn.Func.Inl == nil { - // No inlinable body. + if logopt.Enabled() { + logopt.LogOpt(n.Pos, "cannotInlineCall", "inline", Curfn.funcname(), + fmt.Sprintf("%s cannot be inlined", fn.pkgFuncName())) + } return n } if fn.Func.Inl.Cost > maxCost { @@ -896,9 +911,6 @@ func mkinlcall(n, fn *Node, maxCost int32, inlMap map[*Node]bool) *Node { if Debug['m'] > 2 { fmt.Printf("%v: Before inlining: %+v\n", n.Line(), n) } - if logopt.Enabled() { - logopt.LogOpt(n.Pos, "inlineCall", "inline", Curfn.funcname(), fn.pkgFuncName()) - } if ssaDump != "" && ssaDump == Curfn.funcname() { ssaDumpInlined = append(ssaDumpInlined, fn) diff --git a/src/cmd/compile/internal/logopt/logopt_test.go b/src/cmd/compile/internal/logopt/logopt_test.go index cc28536fd4..9704bc79d5 100644 --- a/src/cmd/compile/internal/logopt/logopt_test.go +++ b/src/cmd/compile/internal/logopt/logopt_test.go @@ -30,6 +30,14 @@ func foo(w, z *pair) *int { } return &a[0] } + +// address taking prevents closure inlining +func n() int { + foo := func() int { return 1 } + bar := &foo + x := (*bar)() + foo() + return x +} ` func want(t *testing.T, out string, desired string) { @@ -164,12 +172,13 @@ func s15a8(x *[15]int64) [15]int64 { // All this delicacy with uriIfy and filepath.Join is to get this test to work right on Windows. slogged := normalize(logged, string(uriIfy(dir)), string(uriIfy("tmpdir"))) t.Logf("%s", slogged) - // below shows proper inlining and nilcheck - want(t, slogged, `{"range":{"start":{"line":9,"character":13},"end":{"line":9,"character":13}},"severity":3,"code":"nilcheck","source":"go compiler","message":"","relatedInformation":[{"location":{"uri":"file://tmpdir/file.go","range":{"start":{"line":4,"character":11},"end":{"line":4,"character":11}}},"message":"inlineLoc"}]}`) + // below shows proper nilcheck + want(t, slogged, `{"range":{"start":{"line":9,"character":13},"end":{"line":9,"character":13}},"severity":3,"code":"nilcheck","source":"go compiler","message":"",`+ + `"relatedInformation":[{"location":{"uri":"file://tmpdir/file.go","range":{"start":{"line":4,"character":11},"end":{"line":4,"character":11}}},"message":"inlineLoc"}]}`) want(t, slogged, `{"range":{"start":{"line":11,"character":6},"end":{"line":11,"character":6}},"severity":3,"code":"isInBounds","source":"go compiler","message":""}`) want(t, slogged, `{"range":{"start":{"line":7,"character":6},"end":{"line":7,"character":6}},"severity":3,"code":"canInlineFunction","source":"go compiler","message":"cost: 35"}`) - want(t, slogged, `{"range":{"start":{"line":9,"character":13},"end":{"line":9,"character":13}},"severity":3,"code":"inlineCall","source":"go compiler","message":"x.bar"}`) - want(t, slogged, `{"range":{"start":{"line":8,"character":9},"end":{"line":8,"character":9}},"severity":3,"code":"inlineCall","source":"go compiler","message":"x.bar"}`) + want(t, slogged, `{"range":{"start":{"line":21,"character":21},"end":{"line":21,"character":21}},"severity":3,"code":"cannotInlineCall","source":"go compiler","message":"foo cannot be inlined (escaping closure variable)"}`) + }) } -- 2.50.0