Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

cmd/compile: redundant information produced in compiler optimization debug output #46328

Closed
go101 opened this issue May 23, 2021 · 11 comments
Closed
Milestone

Comments

@go101
Copy link

@go101 go101 commented May 23, 2021

What version of Go are you using (go version)?

$ go version
go version go1.16.3 linux/amd64

Does this issue reproduce with the latest release?

Yes

What did you do?

package main

type T struct {
	X int
	Y *int
}

func f(t *T) {
	t.Y = &t.X
}

func g() {
	var t T // 13:6: moved to heap: t
	f(&t)
	println(t.X, t.Y)
}

func h() {
	var t T
	t.Y = &t.X
	println(t.X, t.Y)
}

func m() {
	var t T // 25:6: moved to heap: t
	func() { t.Y = &t.X }() // or: func(v *T) { v.Y = &v.X }(&t)
	println(t.X, t.Y)
}

func main() {
	g() // 31:3: moved to heap: t
	m() // (no message for this line?)
}

What did you expect to see?

Either remove the message for line 31, or output the similar message for line 32.
The former way is recommended, because the current outputs is too much.

What did you see instead?

$ go build -gcflags=-m a.go
# command-line-arguments
./a.go:8:6: can inline f
./a.go:12:6: can inline g
./a.go:14:3: inlining call to f
./a.go:18:6: can inline h
./a.go:26:2: can inline m.func1
./a.go:26:23: inlining call to m.func1
./a.go:30:6: can inline main
./a.go:31:3: inlining call to g
./a.go:31:3: inlining call to f
./a.go:8:8: leaking param: t
./a.go:13:6: moved to heap: t
./a.go:25:6: moved to heap: t
./a.go:31:3: moved to heap: t
@go101
Copy link
Author

@go101 go101 commented May 23, 2021

It looks the difference here is g is inlined but m is not.
However, the message at line 31 still looks redundant (and some confusing).

Loading

@go101
Copy link
Author

@go101 go101 commented May 23, 2021

Another example, in which it is NOT (edited) clear whether or not the g function will allocate the slice on heap.

package main

type T int
const N = 1<<12
var i = N - 1

func main() {
    var r1 = g() // make([]T, N) does not escape
    println(r1[i])
   
}

func g() []T {
    var ts = make([]T, N) // make([]T, N) escapes to heap
    return ts
}
$ go build -gcflags=-m a.go
# command-line-arguments
./a.go:14:6: can inline g
./a.go:8:6: can inline main
./a.go:9:15: inlining call to g
./a.go:9:15: make([]T, N) does not escape
./a.go:15:18: make([]T, N) escapes to heap

Loading

@mknyszek
Copy link
Contributor

@mknyszek mknyszek commented May 24, 2021

It looks like you're passing the -m flag to cmd/compile? Can you please update your post to reflect that and show the steps you took to generate that output? (e.g. go build -gcflags=-m ... or whatever it was you did).

IIUC, this is mainly compiler debug output. I'm all for making that better, but I think in this particular case you do want to know both things. Looking at your most recent example, I think it's saying that the inlined version doesn't cause an allocation while the non-inlined version does. Both pieces of information are useful, not redundant.

CC @mdempsky @dr2chase

Loading

@mknyszek mknyszek added this to the Backlog milestone May 24, 2021
@mknyszek mknyszek changed the title cmd/compile: inconsistent message outputs cmd/compile: redundant information produced in compiler optimization debug output May 24, 2021
@go101
Copy link
Author

@go101 go101 commented May 24, 2021

Thanks for the explanation.

Personally, I think brings some noises and misunderstanding.
For this simple example, the misunderstanding is small.
For a real large project, people might not notice the messages at call sites.

Loading

@mknyszek
Copy link
Contributor

@mknyszek mknyszek commented May 24, 2021

Thanks for the explanation.

Personally, I think brings some noises and misunderstanding.
For this simple example, the misunderstanding is small.
For a real large project, people might not notice the messages at call sites.

Generally speaking, I think if someone is trying to work with the compiler on these optimizations, these sorts of situations would be more clear to them. And again, I think more information here is better than less.

For large projects, it might be better to use the JSON output (-json flag) for compiler optimizations anyway, and use an automated tool to ensure optimizations are applicable on hot paths anyway. Otherwise changes to code could make the optimization accidentally disappear over time anyway (benchmarks are not perfect for this because often they enable additional inlining into the benchmark itself that may not apply in real-world use-cases).

Loading

@dr2chase
Copy link
Contributor

@dr2chase dr2chase commented May 24, 2021

More on -json:

To get this information, pass -gcflags=-json=0,<some absolute directory path> on the command line, for example -gcflags=-json=0,$PWD/foo.lspdir which would create foo.lspdir in the current directory and fill it with Language-Server-Protocol JSON files describing missed optimizations, including all the inlining information.

The format is described here: https://go.googlesource.com/go/+/refs/heads/master/src/cmd/compile/internal/logopt/log_opts.go#24

I wrote a proof of concept for combining this with profile output, https://github.com/dr2chase/gc-lsp-tools .

Loading

@go101
Copy link
Author

@go101 go101 commented May 24, 2021

And again, I think more information here is better than less.

Sometimes, not always.

BTW, do all the messages at every call sites of the same inlined function are the same?
If the answer is true, then it would be great if an option could be provided to

  • not show the messages if an inline-able function is not inlined
  • not show the messages inside an inline-able function at all call sites.

Like:

$ go build -gcflags=-m a.go
# command-line-arguments
./a.go:8:6: can inline main
./a.go:9:15: inlining call to g
./a.go:14:6: can inline g
./a.go:15:18: make([]T, N) does not escape

Loading

@dr2chase
Copy link
Contributor

@dr2chase dr2chase commented May 24, 2021

Again, if this is important to you, I recommend looking at the -json output. We're very unlikely to add more compiler flags to tweak the output of -m.

Loading

@go101
Copy link
Author

@go101 go101 commented May 24, 2021

OK. Thanks for letting me know the -json option. It might be very useful when I study some Go code later.

Loading

@go101
Copy link
Author

@go101 go101 commented May 25, 2021

do all the messages at every call sites of the same inlined function are the same?

The answer should be false. So got it.

Loading

@go101
Copy link
Author

@go101 go101 commented May 26, 2021

It looks no changes needed to be made here. So close it.

Loading

@go101 go101 closed this May 26, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
3 participants