Commit cd53fdda authored by David Chase's avatar David Chase

cmd/compile: add framework for logging optimizer (non)actions to LSP

This is intended to allow IDEs to note where the optimizer
was not able to improve users' code.  There may be other
applications for this, for example in studying effectiveness
of optimizer changes more quickly than running benchmarks,
or in verifying that code changes did not accidentally disable
optimizations in performance-critical code.

Logging of nilcheck (bad) for amd64 is implemented as
proof-of-concept.  In general, the intent is that optimizations
that didn't happen are what will be logged, because that is
believed to be what IDE users want.

Added flag -json=version,dest

Check that version=0.  (Future compilers will support a
few recent versions, I hope that version is always <=3.)

Dest is expected to be one of:

/path (or \path in Windows)
  will create directory /path and fill it w/ json files
file://path
  will create directory path, intended either for
     I:\dont\know\enough\about\windows\paths
     trustme_I_know_what_I_am_doing_probably_testing

Not passing an absolute path name usually leads to
json splattered all over source directories,
or failure when those directories are not writeable.
If you want a foot-gun, you have to ask for it.

The JSON output is directed to subdirectories of dest,
where each subdirectory is net/url.PathEscape of the
package name, and each for each foo.go in the package,
net/url.PathEscape(foo).json is created.  The first line
of foo.json contains version and context information,
and subsequent lines contains LSP-conforming JSON
describing the missing optimizations.

Change-Id: Ib83176a53a8c177ee9081aefc5ae05604ccad8a0
Reviewed-on: https://go-review.googlesource.com/c/go/+/204338
Run-TryBot: David Chase <drchase@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: default avatarCherry Zhang <cherryyz@google.com>
parent 4d4ddd86
...@@ -5,6 +5,7 @@ ...@@ -5,6 +5,7 @@
package amd64 package amd64
import ( import (
"cmd/compile/internal/logopt"
"fmt" "fmt"
"math" "math"
...@@ -1081,6 +1082,9 @@ func ssaGenValue(s *gc.SSAGenState, v *ssa.Value) { ...@@ -1081,6 +1082,9 @@ func ssaGenValue(s *gc.SSAGenState, v *ssa.Value) {
p.To.Type = obj.TYPE_MEM p.To.Type = obj.TYPE_MEM
p.To.Reg = v.Args[0].Reg() p.To.Reg = v.Args[0].Reg()
gc.AddAux(&p.To, v) gc.AddAux(&p.To, v)
if logopt.Enabled() {
logopt.LogOpt(v.Pos, "nilcheck", "genssa", v.Block.Func.Name)
}
if gc.Debug_checknil != 0 && v.Pos.Line() > 1 { // v.Pos.Line()==1 in generated wrappers if gc.Debug_checknil != 0 && v.Pos.Line() > 1 { // v.Pos.Line()==1 in generated wrappers
gc.Warnl(v.Pos, "generated nil check") gc.Warnl(v.Pos, "generated nil check")
} }
......
...@@ -9,6 +9,7 @@ package gc ...@@ -9,6 +9,7 @@ package gc
import ( import (
"bufio" "bufio"
"bytes" "bytes"
"cmd/compile/internal/logopt"
"cmd/compile/internal/ssa" "cmd/compile/internal/ssa"
"cmd/compile/internal/types" "cmd/compile/internal/types"
"cmd/internal/bio" "cmd/internal/bio"
...@@ -203,6 +204,7 @@ func Main(archInit func(*Arch)) { ...@@ -203,6 +204,7 @@ func Main(archInit func(*Arch)) {
// Whether the limit for stack-allocated objects is much smaller than normal. // Whether the limit for stack-allocated objects is much smaller than normal.
// This can be helpful for diagnosing certain causes of GC latency. See #27732. // This can be helpful for diagnosing certain causes of GC latency. See #27732.
smallFrames := false smallFrames := false
jsonLogOpt := ""
flag.BoolVar(&compiling_runtime, "+", false, "compiling runtime") flag.BoolVar(&compiling_runtime, "+", false, "compiling runtime")
flag.BoolVar(&compiling_std, "std", false, "compiling standard library") flag.BoolVar(&compiling_std, "std", false, "compiling standard library")
...@@ -276,6 +278,7 @@ func Main(archInit func(*Arch)) { ...@@ -276,6 +278,7 @@ func Main(archInit func(*Arch)) {
flag.BoolVar(&smallFrames, "smallframes", false, "reduce the size limit for stack allocated objects") flag.BoolVar(&smallFrames, "smallframes", false, "reduce the size limit for stack allocated objects")
flag.BoolVar(&Ctxt.UseBASEntries, "dwarfbasentries", Ctxt.UseBASEntries, "use base address selection entries in DWARF") flag.BoolVar(&Ctxt.UseBASEntries, "dwarfbasentries", Ctxt.UseBASEntries, "use base address selection entries in DWARF")
flag.BoolVar(&Ctxt.Flag_newobj, "newobj", false, "use new object file format") flag.BoolVar(&Ctxt.Flag_newobj, "newobj", false, "use new object file format")
flag.StringVar(&jsonLogOpt, "json", "", "version,destination for JSON compiler/optimizer logging")
objabi.Flagparse(usage) objabi.Flagparse(usage)
...@@ -478,6 +481,10 @@ func Main(archInit func(*Arch)) { ...@@ -478,6 +481,10 @@ func Main(archInit func(*Arch)) {
Debug['l'] = 1 - Debug['l'] Debug['l'] = 1 - Debug['l']
} }
if jsonLogOpt != "" { // parse version,destination from json logging optimization.
logopt.LogJsonOption(jsonLogOpt)
}
ssaDump = os.Getenv("GOSSAFUNC") ssaDump = os.Getenv("GOSSAFUNC")
if ssaDump != "" { if ssaDump != "" {
if strings.HasSuffix(ssaDump, "+") { if strings.HasSuffix(ssaDump, "+") {
...@@ -772,6 +779,8 @@ func Main(archInit func(*Arch)) { ...@@ -772,6 +779,8 @@ func Main(archInit func(*Arch)) {
Fatalf("%d uncompiled functions", len(compilequeue)) Fatalf("%d uncompiled functions", len(compilequeue))
} }
logopt.FlushLoggedOpts(Ctxt, myimportpath)
if nerrors+nsavederrors != 0 { if nerrors+nsavederrors != 0 {
errorexit() errorexit()
} }
......
...@@ -5,14 +5,14 @@ ...@@ -5,14 +5,14 @@
package gc package gc
import ( import (
"bufio"
"bytes"
"encoding/binary" "encoding/binary"
"fmt" "fmt"
"html" "html"
"os" "os"
"sort" "sort"
"bufio"
"bytes"
"cmd/compile/internal/ssa" "cmd/compile/internal/ssa"
"cmd/compile/internal/types" "cmd/compile/internal/types"
"cmd/internal/obj" "cmd/internal/obj"
......
// Copyright 2019 The Go Authors. All rights reserved.
// Use of this source code is governed by a BSD-style
// license that can be found in the LICENSE file.
// +build go1.8
package logopt
import "net/url"
func pathEscape(s string) string {
return url.PathEscape(s)
}
// Copyright 2019 The Go Authors. All rights reserved.
// Use of this source code is governed by a BSD-style
// license that can be found in the LICENSE file.
// +build !go1.8
package logopt
// For bootstrapping with an early version of Go
func pathEscape(s string) string {
panic("This should never be called; the compiler is not fully bootstrapped if it is.")
}
This diff is collapsed.
// Copyright 2019 The Go Authors. All rights reserved.
// Use of this source code is governed by a BSD-style
// license that can be found in the LICENSE file.
package logopt
import (
"internal/testenv"
"io/ioutil"
"os"
"os/exec"
"path/filepath"
"runtime"
"strings"
"testing"
)
const srcCode = `package x
type pair struct {a,b int}
func bar(y *pair) *int {
return &y.b
}
func foo(w, z *pair) *int {
if *bar(w) > 0 {
return bar(z)
}
return nil
}
`
func want(t *testing.T, out string, desired string) {
if !strings.Contains(out, desired) {
t.Errorf("did not see phrase %s in \n%s", desired, out)
}
}
func TestLogOpt(t *testing.T) {
t.Parallel()
testenv.MustHaveGoBuild(t)
dir, err := ioutil.TempDir("", "TestLogOpt")
if err != nil {
t.Fatal(err)
}
defer os.RemoveAll(dir)
dir = fixSlash(dir) // Normalize the directory name as much as possible, for Windows testing
src := filepath.Join(dir, "file.go")
if err := ioutil.WriteFile(src, []byte(srcCode), 0644); err != nil {
t.Fatal(err)
}
outfile := filepath.Join(dir, "file.o")
t.Run("JSON_fails", func(t *testing.T) {
// Test malformed flag
out, err := testLogOpt(t, "-json=foo", src, outfile)
if err == nil {
t.Error("-json=foo succeeded unexpectedly")
}
want(t, out, "option should be")
want(t, out, "number")
// Test a version number that is currently unsupported (and should remain unsupported for a while)
out, err = testLogOpt(t, "-json=9,foo", src, outfile)
if err == nil {
t.Error("-json=0,foo succeeded unexpectedly")
}
want(t, out, "version must be")
})
// Some architectures don't fault on nil dereference, so nilchecks are eliminated differently.
if runtime.GOARCH != "amd64" {
return
}
t.Run("Success", func(t *testing.T) {
// This test is supposed to succeed
// replace d (dir) with t ("tmpdir") and convert path separators to '/'
normalize := func(out []byte, d, t string) string {
s := string(out)
s = strings.ReplaceAll(s, d, t)
s = strings.ReplaceAll(s, string(os.PathSeparator), "/")
return s
}
// Note 'file://' is the I-Know-What-I-Am-Doing way of specifying a file, also to deal with corner cases for Windows.
_, err := testLogOptDir(t, dir, "-json=0,file://log/opt", src, outfile)
if err != nil {
t.Error("-json=0,file://log/opt should have succeeded")
}
logged, err := ioutil.ReadFile(filepath.Join(dir, "log", "opt", "x", "file.json"))
if err != nil {
t.Error("-json=0,file://log/opt missing expected log file")
}
// 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"}]}`)
})
}
func testLogOpt(t *testing.T, flag, src, outfile string) (string, error) {
run := []string{testenv.GoToolPath(t), "tool", "compile", flag, "-o", outfile, src}
t.Log(run)
cmd := exec.Command(run[0], run[1:]...)
out, err := cmd.CombinedOutput()
t.Logf("%s", out)
return string(out), err
}
func testLogOptDir(t *testing.T, dir, flag, src, outfile string) (string, error) {
// Notice the specified import path "x"
run := []string{testenv.GoToolPath(t), "tool", "compile", "-p", "x", flag, "-o", outfile, src}
t.Log(run)
cmd := exec.Command(run[0], run[1:]...)
cmd.Dir = dir
out, err := cmd.CombinedOutput()
t.Logf("%s", out)
return string(out), err
}
...@@ -41,6 +41,7 @@ var bootstrapDirs = []string{ ...@@ -41,6 +41,7 @@ var bootstrapDirs = []string{
"cmd/compile/internal/arm", "cmd/compile/internal/arm",
"cmd/compile/internal/arm64", "cmd/compile/internal/arm64",
"cmd/compile/internal/gc", "cmd/compile/internal/gc",
"cmd/compile/internal/logopt",
"cmd/compile/internal/mips", "cmd/compile/internal/mips",
"cmd/compile/internal/mips64", "cmd/compile/internal/mips64",
"cmd/compile/internal/ppc64", "cmd/compile/internal/ppc64",
......
...@@ -108,6 +108,21 @@ func (ctxt *Link) InnermostPos(xpos src.XPos) src.Pos { ...@@ -108,6 +108,21 @@ func (ctxt *Link) InnermostPos(xpos src.XPos) src.Pos {
return ctxt.PosTable.Pos(xpos) return ctxt.PosTable.Pos(xpos)
} }
// AllPos returns a slice of the positions inlined at xpos, from
// innermost (index zero) to outermost. To avoid gratuitous allocation
// the result is passed in and extended if necessary.
func (ctxt *Link) AllPos(xpos src.XPos, result []src.Pos) []src.Pos {
pos := ctxt.InnermostPos(xpos)
result = result[:0]
result = append(result, ctxt.PosTable.Pos(xpos))
for ix := pos.Base().InliningIndex(); ix >= 0; {
call := ctxt.InlTree.nodes[ix]
ix = call.Parent
result = append(result, ctxt.PosTable.Pos(call.Pos))
}
return result
}
func dumpInlTree(ctxt *Link, tree InlTree) { func dumpInlTree(ctxt *Link, tree InlTree) {
for i, call := range tree.nodes { for i, call := range tree.nodes {
pos := ctxt.PosTable.Pos(call.Pos) pos := ctxt.PosTable.Pos(call.Pos)
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment