cmd/compile/internal/devirtualize: devirtualize methods in other packages if current package has a concrete reference

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: 2d53c55
GitHub-Pull-Request: #60565
Reviewed-on: https://go-review.googlesource.com/c/go/+/500155
Run-TryBot: thepudds <[email protected]>
Reviewed-by: Cherry Mui <[email protected]>
Reviewed-by: Michael Pratt <[email protected]>
TryBot-Result: Gopher Robot <[email protected]>

Author: thepudds

src/cmd/compile/internal/devirtualize/pgo.go

@@ -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))
 	}

src/cmd/compile/internal/inline/inl.go

@@ -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
 	}

src/cmd/compile/internal/test/pgo_devirtualize_test.go

@@ -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)
 		}

src/cmd/compile/internal/test/testdata/pgo/devirtualize/devirt.go

@@ -11,32 +11,12 @@
 
 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)
+}

src/cmd/compile/internal/test/testdata/pgo/devirtualize/devirt.pprof

@@ -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)

src/cmd/compile/internal/test/testdata/pgo/devirtualize/devirt_test.go

@@ -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
+}