aboutsummaryrefslogtreecommitdiff
diff options
context:
space:
mode:
-rw-r--r--src/cmd/compile/internal/noder/reader.go53
-rw-r--r--src/cmd/compile/internal/noder/unified.go12
-rw-r--r--src/cmd/compile/internal/test/pgo_devirtualize_test.go181
3 files changed, 180 insertions, 66 deletions
diff --git a/src/cmd/compile/internal/noder/reader.go b/src/cmd/compile/internal/noder/reader.go
index f5d1fce50c..2dddd20165 100644
--- a/src/cmd/compile/internal/noder/reader.go
+++ b/src/cmd/compile/internal/noder/reader.go
@@ -663,9 +663,24 @@ func (pr *pkgReader) objInstIdx(info objInfo, dict *readerDict, shaped bool) ir.
}
// objIdx returns the specified object, instantiated with the given
-// type arguments, if any. If shaped is true, then the shaped variant
-// of the object is returned instead.
+// type arguments, if any.
+// If shaped is true, then the shaped variant of the object is returned
+// instead.
func (pr *pkgReader) objIdx(idx pkgbits.Index, implicits, explicits []*types.Type, shaped bool) ir.Node {
+ n, err := pr.objIdxMayFail(idx, implicits, explicits, shaped)
+ if err != nil {
+ base.Fatalf("%v", err)
+ }
+ return n
+}
+
+// objIdxMayFail is equivalent to objIdx, but returns an error rather than
+// failing the build if this object requires type arguments and the incorrect
+// number of type arguments were passed.
+//
+// Other sources of internal failure (such as duplicate definitions) still fail
+// the build.
+func (pr *pkgReader) objIdxMayFail(idx pkgbits.Index, implicits, explicits []*types.Type, shaped bool) (ir.Node, error) {
rname := pr.newReader(pkgbits.RelocName, idx, pkgbits.SyncObject1)
_, sym := rname.qualifiedIdent()
tag := pkgbits.CodeObj(rname.Code(pkgbits.SyncCodeObj))
@@ -674,22 +689,25 @@ func (pr *pkgReader) objIdx(idx pkgbits.Index, implicits, explicits []*types.Typ
assert(!sym.IsBlank())
switch sym.Pkg {
case types.BuiltinPkg, types.UnsafePkg:
- return sym.Def.(ir.Node)
+ return sym.Def.(ir.Node), nil
}
if pri, ok := objReader[sym]; ok {
- return pri.pr.objIdx(pri.idx, nil, explicits, shaped)
+ return pri.pr.objIdxMayFail(pri.idx, nil, explicits, shaped)
}
if sym.Pkg.Path == "runtime" {
- return typecheck.LookupRuntime(sym.Name)
+ return typecheck.LookupRuntime(sym.Name), nil
}
base.Fatalf("unresolved stub: %v", sym)
}
- dict := pr.objDictIdx(sym, idx, implicits, explicits, shaped)
+ dict, err := pr.objDictIdx(sym, idx, implicits, explicits, shaped)
+ if err != nil {
+ return nil, err
+ }
sym = dict.baseSym
if !sym.IsBlank() && sym.Def != nil {
- return sym.Def.(*ir.Name)
+ return sym.Def.(*ir.Name), nil
}
r := pr.newReader(pkgbits.RelocObj, idx, pkgbits.SyncObject1)
@@ -725,7 +743,7 @@ func (pr *pkgReader) objIdx(idx pkgbits.Index, implicits, explicits []*types.Typ
name := do(ir.OTYPE, false)
setType(name, r.typ())
name.SetAlias(true)
- return name
+ return name, nil
case pkgbits.ObjConst:
name := do(ir.OLITERAL, false)
@@ -733,7 +751,7 @@ func (pr *pkgReader) objIdx(idx pkgbits.Index, implicits, explicits []*types.Typ
val := FixValue(typ, r.Value())
setType(name, typ)
setValue(name, val)
- return name
+ return name, nil
case pkgbits.ObjFunc:
if sym.Name == "init" {
@@ -768,7 +786,7 @@ func (pr *pkgReader) objIdx(idx pkgbits.Index, implicits, explicits []*types.Typ
}
rext.funcExt(name, nil)
- return name
+ return name, nil
case pkgbits.ObjType:
name := do(ir.OTYPE, true)
@@ -805,13 +823,13 @@ func (pr *pkgReader) objIdx(idx pkgbits.Index, implicits, explicits []*types.Typ
r.needWrapper(typ)
}
- return name
+ return name, nil
case pkgbits.ObjVar:
name := do(ir.ONAME, false)
setType(name, r.typ())
rext.varExt(name)
- return name
+ return name, nil
}
}
@@ -908,7 +926,7 @@ func shapify(targ *types.Type, basic bool) *types.Type {
}
// objDictIdx reads and returns the specified object dictionary.
-func (pr *pkgReader) objDictIdx(sym *types.Sym, idx pkgbits.Index, implicits, explicits []*types.Type, shaped bool) *readerDict {
+func (pr *pkgReader) objDictIdx(sym *types.Sym, idx pkgbits.Index, implicits, explicits []*types.Type, shaped bool) (*readerDict, error) {
r := pr.newReader(pkgbits.RelocObjDict, idx, pkgbits.SyncObject1)
dict := readerDict{
@@ -919,7 +937,7 @@ func (pr *pkgReader) objDictIdx(sym *types.Sym, idx pkgbits.Index, implicits, ex
nexplicits := r.Len()
if nimplicits > len(implicits) || nexplicits != len(explicits) {
- base.Fatalf("%v has %v+%v params, but instantiated with %v+%v args", sym, nimplicits, nexplicits, len(implicits), len(explicits))
+ return nil, fmt.Errorf("%v has %v+%v params, but instantiated with %v+%v args", sym, nimplicits, nexplicits, len(implicits), len(explicits))
}
dict.targs = append(implicits[:nimplicits:nimplicits], explicits...)
@@ -984,7 +1002,7 @@ func (pr *pkgReader) objDictIdx(sym *types.Sym, idx pkgbits.Index, implicits, ex
dict.itabs[i] = itabInfo{typ: r.typInfo(), iface: r.typInfo()}
}
- return &dict
+ return &dict, nil
}
func (r *reader) typeParamNames() {
@@ -2529,7 +2547,10 @@ func (pr *pkgReader) objDictName(idx pkgbits.Index, implicits, explicits []*type
base.Fatalf("unresolved stub: %v", sym)
}
- dict := pr.objDictIdx(sym, idx, implicits, explicits, false)
+ dict, err := pr.objDictIdx(sym, idx, implicits, explicits, false)
+ if err != nil {
+ base.Fatalf("%v", err)
+ }
return pr.dictNameOf(dict)
}
diff --git a/src/cmd/compile/internal/noder/unified.go b/src/cmd/compile/internal/noder/unified.go
index d2ca1f37a9..492b00d256 100644
--- a/src/cmd/compile/internal/noder/unified.go
+++ b/src/cmd/compile/internal/noder/unified.go
@@ -80,7 +80,11 @@ func lookupFunction(pkg *types.Pkg, symName string) (*ir.Func, error) {
return nil, fmt.Errorf("func sym %v missing objReader", sym)
}
- name := pri.pr.objIdx(pri.idx, nil, nil, false).(*ir.Name)
+ node, err := pri.pr.objIdxMayFail(pri.idx, nil, nil, false)
+ if err != nil {
+ return nil, fmt.Errorf("func sym %v lookup error: %w", sym, err)
+ }
+ name := node.(*ir.Name)
if name.Op() != ir.ONAME || name.Class != ir.PFUNC {
return nil, fmt.Errorf("func sym %v refers to non-function name: %v", sym, name)
}
@@ -105,7 +109,11 @@ func lookupMethod(pkg *types.Pkg, symName string) (*ir.Func, error) {
return nil, fmt.Errorf("type sym %v missing objReader", typ)
}
- name := pri.pr.objIdx(pri.idx, nil, nil, false).(*ir.Name)
+ node, err := pri.pr.objIdxMayFail(pri.idx, nil, nil, false)
+ if err != nil {
+ return nil, fmt.Errorf("func sym %v lookup error: %w", typ, err)
+ }
+ name := node.(*ir.Name)
if name.Op() != ir.OTYPE {
return nil, fmt.Errorf("type sym %v refers to non-type name: %v", typ, name)
}
diff --git a/src/cmd/compile/internal/test/pgo_devirtualize_test.go b/src/cmd/compile/internal/test/pgo_devirtualize_test.go
index c457478a1f..f451243683 100644
--- a/src/cmd/compile/internal/test/pgo_devirtualize_test.go
+++ b/src/cmd/compile/internal/test/pgo_devirtualize_test.go
@@ -14,8 +14,13 @@ import (
"testing"
)
+type devirtualization struct {
+ pos string
+ callee string
+}
+
// testPGODevirtualize tests that specific PGO devirtualize rewrites are performed.
-func testPGODevirtualize(t *testing.T, dir string) {
+func testPGODevirtualize(t *testing.T, dir string, want []devirtualization) {
testenv.MustHaveGoRun(t)
t.Parallel()
@@ -23,7 +28,7 @@ func testPGODevirtualize(t *testing.T, dir string) {
// Add a go.mod so we have a consistent symbol names in this temp dir.
goMod := fmt.Sprintf(`module %s
-go 1.19
+go 1.21
`, pkg)
if err := os.WriteFile(filepath.Join(dir, "go.mod"), []byte(goMod), 0644); err != nil {
t.Fatalf("error writing go.mod: %v", err)
@@ -60,51 +65,6 @@ go 1.19
t.Fatalf("error starting go test: %v", err)
}
- type devirtualization struct {
- pos string
- callee string
- }
-
- want := []devirtualization{
- // ExerciseIface
- {
- pos: "./devirt.go:101:20",
- callee: "mult.Mult.Multiply",
- },
- {
- pos: "./devirt.go:101:39",
- callee: "Add.Add",
- },
- // ExerciseFuncConcrete
- {
- pos: "./devirt.go:173:36",
- callee: "AddFn",
- },
- {
- pos: "./devirt.go:173:15",
- callee: "mult.MultFn",
- },
- // ExerciseFuncField
- {
- pos: "./devirt.go:207:35",
- callee: "AddFn",
- },
- {
- pos: "./devirt.go:207:19",
- callee: "mult.MultFn",
- },
- // ExerciseFuncClosure
- // TODO(prattmic): Closure callees not implemented.
- //{
- // pos: "./devirt.go:249:27",
- // callee: "AddClosure.func1",
- //},
- //{
- // pos: "./devirt.go:249:15",
- // callee: "mult.MultClosure.func1",
- //},
- }
-
got := make(map[devirtualization]struct{})
devirtualizedLine := regexp.MustCompile(`(.*): PGO devirtualizing \w+ call .* to (.*)`)
@@ -172,5 +132,130 @@ func TestPGODevirtualize(t *testing.T) {
}
}
- testPGODevirtualize(t, dir)
+ want := []devirtualization{
+ // ExerciseIface
+ {
+ pos: "./devirt.go:101:20",
+ callee: "mult.Mult.Multiply",
+ },
+ {
+ pos: "./devirt.go:101:39",
+ callee: "Add.Add",
+ },
+ // ExerciseFuncConcrete
+ {
+ pos: "./devirt.go:173:36",
+ callee: "AddFn",
+ },
+ {
+ pos: "./devirt.go:173:15",
+ callee: "mult.MultFn",
+ },
+ // ExerciseFuncField
+ {
+ pos: "./devirt.go:207:35",
+ callee: "AddFn",
+ },
+ {
+ pos: "./devirt.go:207:19",
+ callee: "mult.MultFn",
+ },
+ // ExerciseFuncClosure
+ // TODO(prattmic): Closure callees not implemented.
+ //{
+ // pos: "./devirt.go:249:27",
+ // callee: "AddClosure.func1",
+ //},
+ //{
+ // pos: "./devirt.go:249:15",
+ // callee: "mult.MultClosure.func1",
+ //},
+ }
+
+ testPGODevirtualize(t, dir, want)
+}
+
+// Regression test for https://go.dev/issue/65615. If a target function changes
+// from non-generic to generic we can't devirtualize it (don't know the type
+// parameters), but the compiler should not crash.
+func TestLookupFuncGeneric(t *testing.T) {
+ wd, err := os.Getwd()
+ if err != nil {
+ t.Fatalf("error getting wd: %v", err)
+ }
+ srcDir := filepath.Join(wd, "testdata", "pgo", "devirtualize")
+
+ // Copy the module to a scratch location so we can add a go.mod.
+ dir := t.TempDir()
+ if err := os.Mkdir(filepath.Join(dir, "mult.pkg"), 0755); err != nil {
+ t.Fatalf("error creating dir: %v", err)
+ }
+ for _, file := range []string{"devirt.go", "devirt_test.go", "devirt.pprof", filepath.Join("mult.pkg", "mult.go")} {
+ if err := copyFile(filepath.Join(dir, file), filepath.Join(srcDir, file)); err != nil {
+ t.Fatalf("error copying %s: %v", file, err)
+ }
+ }
+
+ // Change MultFn from a concrete function to a parameterized function.
+ if err := convertMultToGeneric(filepath.Join(dir, "mult.pkg", "mult.go")); err != nil {
+ t.Fatalf("error editing mult.go: %v", err)
+ }
+
+ // Same as TestPGODevirtualize except for MultFn, which we cannot
+ // devirtualize to because it has become generic.
+ //
+ // Note that the important part of this test is that the build is
+ // successful, not the specific devirtualizations.
+ want := []devirtualization{
+ // ExerciseIface
+ {
+ pos: "./devirt.go:101:20",
+ callee: "mult.Mult.Multiply",
+ },
+ {
+ pos: "./devirt.go:101:39",
+ callee: "Add.Add",
+ },
+ // ExerciseFuncConcrete
+ {
+ pos: "./devirt.go:173:36",
+ callee: "AddFn",
+ },
+ // ExerciseFuncField
+ {
+ pos: "./devirt.go:207:35",
+ callee: "AddFn",
+ },
+ // ExerciseFuncClosure
+ // TODO(prattmic): Closure callees not implemented.
+ //{
+ // pos: "./devirt.go:249:27",
+ // callee: "AddClosure.func1",
+ //},
+ //{
+ // pos: "./devirt.go:249:15",
+ // callee: "mult.MultClosure.func1",
+ //},
+ }
+
+ testPGODevirtualize(t, dir, want)
+}
+
+var multFnRe = regexp.MustCompile(`func MultFn\(a, b int64\) int64`)
+
+func convertMultToGeneric(path string) error {
+ content, err := os.ReadFile(path)
+ if err != nil {
+ return fmt.Errorf("error opening: %w", err)
+ }
+
+ if !multFnRe.Match(content) {
+ return fmt.Errorf("MultFn not found; update regexp?")
+ }
+
+ // Users of MultFn shouldn't need adjustment, type inference should
+ // work OK.
+ content = multFnRe.ReplaceAll(content, []byte(`func MultFn[T int32|int64](a, b T) T`))
+
+ return os.WriteFile(path, content, 0644)
}