]> Cypherpunks.ru repositories - gostls13.git/commitdiff
cmd/compile/internal/devirtualize: devirtualize methods in other packages if current...
authorthepudds <thepudds@users.noreply.github.com>
Sat, 3 Jun 2023 00:56:31 +0000 (00:56 +0000)
committerCherry Mui <cherryyz@google.com>
Sat, 3 Jun 2023 01:13:08 +0000 (01:13 +0000)
The new PGO-driven indirect call specialization from CL 492436
in theory should allow for devirtualization on methods
in another package when those methods are directly referenced
in the current package.

However, inline.InlineImpossible was checking for a zero-length
fn.Body and would cause devirtualization to fail
with a debug log message like:

 "should not PGO devirtualize (*Speaker1).Speak: no function body"

Previously, the logic in inline.InlineImpossible was only
called on local functions, but with PGO-based devirtualization,
it can now be called on imported functions, where inlinable
imported functions will have a zero-length fn.Body but a
non-nil fn.Inl.

We update inline.InlineImpossible to handle imported functions
by adding a call to typecheck.HaveInlineBody in the check
that was previously failing.

For the test, we need to have a hopefully temporary workaround
of adding explicit references to the callees in another package
for devirtualization to work. CL 497175 or similar should
enable removing this workaround.

Fixes #60561
Updates #59959

Change-Id: I48449b7d8b329d84151bd3b506b8093c262eb2a3
GitHub-Last-Rev: 2d53c55fd895ad8fefd25510a6e6969e89d54a6d
GitHub-Pull-Request: golang/go#60565
Reviewed-on: https://go-review.googlesource.com/c/go/+/500155
Run-TryBot: thepudds <thepudds1460@gmail.com>
Reviewed-by: Cherry Mui <cherryyz@google.com>
Reviewed-by: Michael Pratt <mpratt@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>

src/cmd/compile/internal/devirtualize/pgo.go
src/cmd/compile/internal/inline/inl.go
src/cmd/compile/internal/test/pgo_devirtualize_test.go
src/cmd/compile/internal/test/testdata/pgo/devirtualize/devirt.go
src/cmd/compile/internal/test/testdata/pgo/devirtualize/devirt.pprof
src/cmd/compile/internal/test/testdata/pgo/devirtualize/devirt_test.go
src/cmd/compile/internal/test/testdata/pgo/devirtualize/mult/mult.go [new file with mode: 0644]

index 69c421ca5af02863b2617936d3f16e28355211f5..979483a46fd83b2b5a788a5088db5876a4bdabe4 100644 (file)
@@ -357,11 +357,11 @@ func rewriteCondCall(call *ir.CallExpr, curfn, callee *ir.Func, concretetyp *typ
                elseBlock.Append(call)
        } else {
                // Copy slice so edits in one location don't affect another.
-               thenRet := append([]ir.Node(nil),  retvars...)
+               thenRet := append([]ir.Node(nil), retvars...)
                thenAsList := ir.NewAssignListStmt(pos, ir.OAS2, thenRet, []ir.Node{concreteCall})
                thenBlock.Append(typecheck.Stmt(thenAsList))
 
-               elseRet := append([]ir.Node(nil),  retvars...)
+               elseRet := append([]ir.Node(nil), retvars...)
                elseAsList := ir.NewAssignListStmt(pos, ir.OAS2, elseRet, []ir.Node{call})
                elseBlock.Append(typecheck.Stmt(elseAsList))
        }
index c61d6d2234253f86dcff7715a69bf7ef2bec5921..4ae7fa95d20c3733ddffff1226aed8edfacf233a 100644 (file)
@@ -414,8 +414,9 @@ func InlineImpossible(fn *ir.Func) string {
                return reason
        }
 
-       // If fn has no body (is defined outside of Go), cannot inline it.
-       if len(fn.Body) == 0 {
+       // If a local function has no fn.Body (is defined outside of Go), cannot inline it.
+       // Imported functions don't have fn.Body but might have inline body in fn.Inl.
+       if len(fn.Body) == 0 && !typecheck.HaveInlineBody(fn) {
                reason = "no function body"
                return reason
        }
index 5ddd6269627a1c4acbdb4b95fd1f7b6cf2c2aee9..d524ddb3a20cbe6f0e15e7d1ee2a338b01b912a8 100644 (file)
@@ -57,11 +57,11 @@ go 1.19
 
        want := []devirtualization{
                {
-                       pos:    "./devirt.go:81:21",
-                       callee: "Mult.Multiply",
+                       pos:    "./devirt.go:61:21",
+                       callee: "mult.Mult.Multiply",
                },
                {
-                       pos:    "./devirt.go:81:31",
+                       pos:    "./devirt.go:61:31",
                        callee: "Add.Add",
                },
        }
@@ -115,8 +115,10 @@ func TestPGODevirtualize(t *testing.T) {
 
        // Copy the module to a scratch location so we can add a go.mod.
        dir := t.TempDir()
-
-       for _, file := range []string{"devirt.go", "devirt_test.go", "devirt.pprof"} {
+       if err := os.Mkdir(filepath.Join(dir, "mult"), 0755); err != nil {
+               t.Fatalf("error creating dir: %v", err)
+       }
+       for _, file := range []string{"devirt.go", "devirt_test.go", "devirt.pprof", filepath.Join("mult", "mult.go")} {
                if err := copyFile(filepath.Join(dir, file), filepath.Join(srcDir, file)); err != nil {
                        t.Fatalf("error copying %s: %v", file, err)
                }
index 3f22093b34779f190952c2f95315c39ef6da5e1e..390b6c350a4cd8ae8e93a8780fcb3918f851cdb7 100644 (file)
 
 package devirt
 
-type Multiplier interface {
-       Multiply(a, b int) int
-}
-
-type Adder interface {
-       Add(a, b int) int
-}
+import "example.com/pgo/devirtualize/mult"
 
 var sink int
 
-type Mult struct{}
-
-func (Mult) Multiply(a, b int) int {
-       for i := 0; i < 1000; i++ {
-               sink++
-       }
-       return a * b
-}
-
-type NegMult struct{}
-
-func (NegMult) Multiply(a, b int) int {
-       for i := 0; i < 1000; i++ {
-               sink++
-       }
-       return -1 * a * b
+type Adder interface {
+       Add(a, b int) int
 }
 
 type Add struct{}
@@ -60,7 +40,7 @@ func (Sub) Add(a, b int) int {
 // Exercise calls mostly a1 and m1.
 //
 //go:noinline
-func Exercise(iter int, a1, a2 Adder, m1, m2 Multiplier) {
+func Exercise(iter int, a1, a2 Adder, m1, m2 mult.Multiplier) {
        for i := 0; i < iter; i++ {
                a := a1
                m := m1
@@ -81,3 +61,13 @@ func Exercise(iter int, a1, a2 Adder, m1, m2 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)
+}
index b72f7cf4b3a7117a8187ed01f81938e8d90c0cbf..5fe5dd606fcd06884036ad130e4cd4e4e1c65c2c 100644 (file)
Binary files a/src/cmd/compile/internal/test/testdata/pgo/devirtualize/devirt.pprof and b/src/cmd/compile/internal/test/testdata/pgo/devirtualize/devirt.pprof differ
index 03c966f6de0d5185bfe5df5b1bb65a48e4d72178..f4cbbb80695458bab3bd91a120a6d86956ae704c 100644 (file)
@@ -13,14 +13,16 @@ package devirt
 
 import (
        "testing"
+
+       "example.com/pgo/devirtualize/mult"
 )
 
 func BenchmarkDevirt(b *testing.B) {
        var (
                a1 Add
                a2 Sub
-               m1 Mult
-               m2 NegMult
+               m1 mult.Mult
+               m2 mult.NegMult
        )
 
        Exercise(b.N, a1, a2, m1, m2)
diff --git a/src/cmd/compile/internal/test/testdata/pgo/devirtualize/mult/mult.go b/src/cmd/compile/internal/test/testdata/pgo/devirtualize/mult/mult.go
new file mode 100644 (file)
index 0000000..8a026a5
--- /dev/null
@@ -0,0 +1,32 @@
+// Copyright 2023 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.
+
+// WARNING: Please avoid updating this file.
+// See the warning in ../devirt.go for more details.
+
+package mult
+
+var sink int
+
+type Multiplier interface {
+       Multiply(a, b int) int
+}
+
+type Mult struct{}
+
+func (Mult) Multiply(a, b int) int {
+       for i := 0; i < 1000; i++ {
+               sink++
+       }
+       return a * b
+}
+
+type NegMult struct{}
+
+func (NegMult) Multiply(a, b int) int {
+       for i := 0; i < 1000; i++ {
+               sink++
+       }
+       return -1 * a * b
+}