diff options
| author | Michael Pratt <mpratt@google.com> | 2023-05-15 18:00:57 -0400 |
|---|---|---|
| committer | Michael Pratt <mpratt@google.com> | 2023-10-13 13:56:32 +0000 |
| commit | bfb8924653526d4c154c678f0d0bc491f60f7fce (patch) | |
| tree | 1e420bc7c6b9bbcf9fdcb074c3f3e2045140019a /src/cmd/compile/internal/test/testdata | |
| parent | 696fb5ead03a4b84e11f2d195bc91838fdd029b2 (diff) | |
| download | go-bfb8924653526d4c154c678f0d0bc491f60f7fce.tar.xz | |
cmd/compile: lookup indirect callees from export data for devirtualization
Today, the PGO IR graph only contains entries for ir.Func loaded into
the package. This can include functions from transitive dependencies,
but only if they happen to be referenced by something in the current
package. If they are not referenced, noder never bothers to load them.
This leads to a deficiency in PGO devirtualization: some callee methods
are available in transitive dependencies but do not devirtualize because
they happen to not get loaded from export data.
Resolve this by adding an explicit lookup from export data of callees
mentioned in the profile.
I have chosen to do this during loading of the profile for simplicity:
the PGO IR graph always contains all of the functions we might need.
That said, it isn't strictly necessary. PGO devirtualization could do
the lookup lazily if it decides it actually needs a method. This saves
work at the expense of a bit more complexity, but I've chosen the
simpler approach for now as I measured the cost of this as significantly
less than the rest of PGO loading.
For #61577.
Change-Id: Ieafb2a549510587027270ee6b4c3aefd149a901f
Reviewed-on: https://go-review.googlesource.com/c/go/+/497175
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
Reviewed-by: Cherry Mui <cherryyz@google.com>
Diffstat (limited to 'src/cmd/compile/internal/test/testdata')
| -rw-r--r-- | src/cmd/compile/internal/test/testdata/pgo/devirtualize/devirt.go | 17 | ||||
| -rw-r--r-- | src/cmd/compile/internal/test/testdata/pgo/devirtualize/devirt.pprof | bin | 699 -> 890 bytes | |||
| -rw-r--r-- | src/cmd/compile/internal/test/testdata/pgo/devirtualize/devirt_test.go | 2 | ||||
| -rw-r--r-- | src/cmd/compile/internal/test/testdata/pgo/devirtualize/mult.pkg/mult.go (renamed from src/cmd/compile/internal/test/testdata/pgo/devirtualize/mult/mult.go) | 0 |
4 files changed, 7 insertions, 12 deletions
diff --git a/src/cmd/compile/internal/test/testdata/pgo/devirtualize/devirt.go b/src/cmd/compile/internal/test/testdata/pgo/devirtualize/devirt.go index 390b6c350a..4748e19e10 100644 --- a/src/cmd/compile/internal/test/testdata/pgo/devirtualize/devirt.go +++ b/src/cmd/compile/internal/test/testdata/pgo/devirtualize/devirt.go @@ -11,7 +11,12 @@ package devirt -import "example.com/pgo/devirtualize/mult" +// Devirtualization of callees from transitive dependencies should work even if +// they aren't directly referenced in the package. See #61577. +// +// Dots in the last package path component are escaped in symbol names. Use one +// to ensure the escaping doesn't break lookup. +import "example.com/pgo/devirtualize/mult.pkg" var sink int @@ -61,13 +66,3 @@ func Exercise(iter int, a1, a2 Adder, m1, m2 mult.Multiplier) { sink += m.Multiply(42, a.Add(1, 2)) } } - -func init() { - // TODO: until https://golang.org/cl/497175 or similar lands, - // we need to create an explicit reference to callees - // in another package for devirtualization to work. - m := mult.Mult{} - m.Multiply(42, 0) - n := mult.NegMult{} - n.Multiply(42, 0) -} diff --git a/src/cmd/compile/internal/test/testdata/pgo/devirtualize/devirt.pprof b/src/cmd/compile/internal/test/testdata/pgo/devirtualize/devirt.pprof Binary files differindex 5fe5dd606f..87e7b62736 100644 --- a/src/cmd/compile/internal/test/testdata/pgo/devirtualize/devirt.pprof +++ b/src/cmd/compile/internal/test/testdata/pgo/devirtualize/devirt.pprof diff --git a/src/cmd/compile/internal/test/testdata/pgo/devirtualize/devirt_test.go b/src/cmd/compile/internal/test/testdata/pgo/devirtualize/devirt_test.go index f4cbbb8069..ef637a876b 100644 --- a/src/cmd/compile/internal/test/testdata/pgo/devirtualize/devirt_test.go +++ b/src/cmd/compile/internal/test/testdata/pgo/devirtualize/devirt_test.go @@ -14,7 +14,7 @@ package devirt import ( "testing" - "example.com/pgo/devirtualize/mult" + "example.com/pgo/devirtualize/mult.pkg" ) func BenchmarkDevirt(b *testing.B) { diff --git a/src/cmd/compile/internal/test/testdata/pgo/devirtualize/mult/mult.go b/src/cmd/compile/internal/test/testdata/pgo/devirtualize/mult.pkg/mult.go index 8a026a52f5..8a026a52f5 100644 --- a/src/cmd/compile/internal/test/testdata/pgo/devirtualize/mult/mult.go +++ b/src/cmd/compile/internal/test/testdata/pgo/devirtualize/mult.pkg/mult.go |
