cmd/compile: update the incorrect assignment of call site offset.

The call site calculation in the previous version is incorrect. For
the PGO preprocess file, the compiler should directly use the call
site offset value. Additionly, this change refactors the preprocess
tool to clean up unused fields including startline, the flat and the
cum.

Change-Id: I7bffed3215d4c016d9a9e4034bfd373bf50ab43f
Reviewed-on: https://go-review.googlesource.com/c/go/+/562795
Reviewed-by: Cherry Mui <[email protected]>
LUCI-TryBot-Result: Go LUCI <[email protected]>
Reviewed-by: Michael Pratt <[email protected]>

Author: jinlin-bayarea

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

@@ -108,7 +108,6 @@ type NamedCallEdge struct {
 	CallerName     string
 	CalleeName     string
 	CallSiteOffset int // Line offset from function start line.
-	CallStartLine  int // Start line of the function. Can be 0 which means missing.
 }
 
 // NamedEdgeMap contains all unique call edges in the profile and their
@@ -336,20 +335,19 @@ func createNamedEdgeMapFromPreprocess(r io.Reader) (edgeMap NamedEdgeMap, totalW
 
 		split := strings.Split(readStr, " ")
 
-		if len(split) != 5 {
-			return NamedEdgeMap{}, 0, fmt.Errorf("preprocessed profile entry got %v want 5 fields", split)
+		if len(split) != 2 {
+			return NamedEdgeMap{}, 0, fmt.Errorf("preprocessed profile entry got %v want 2 fields", split)
 		}
 
 		co, _ := strconv.Atoi(split[0])
-		cs, _ := strconv.Atoi(split[1])
 
 		namedEdge := NamedCallEdge{
 			CallerName:     callerName,
-			CallSiteOffset: co - cs,
+			CalleeName:     calleeName,
+			CallSiteOffset: co,
 		}
 
-		namedEdge.CalleeName = calleeName
-		EWeight, _ := strconv.ParseInt(split[4], 10, 64)
+		EWeight, _ := strconv.ParseInt(split[1], 10, 64)
 
 		weight[namedEdge] += EWeight
 		totalWeight += EWeight

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

@@ -19,8 +19,11 @@ type devirtualization struct {
 	callee string
 }
 
+const profFileName = "devirt.pprof"
+const preProfFileName = "devirt.pprof.node_map"
+
 // testPGODevirtualize tests that specific PGO devirtualize rewrites are performed.
-func testPGODevirtualize(t *testing.T, dir string, want []devirtualization) {
+func testPGODevirtualize(t *testing.T, dir string, want []devirtualization, pgoProfileName string) {
 	testenv.MustHaveGoRun(t)
 	t.Parallel()
 
@@ -45,7 +48,7 @@ go 1.21
 	}
 
 	// Build the test with the profile.
-	pprof := filepath.Join(dir, "devirt.pprof")
+	pprof := filepath.Join(dir, pgoProfileName)
 	gcflag := fmt.Sprintf("-gcflags=-m=2 -pgoprofile=%s -d=pgodebug=3", pprof)
 	out := filepath.Join(dir, "test.exe")
 	cmd = testenv.CleanCmdEnv(testenv.Command(t, testenv.GoToolPath(t), "test", "-o", out, gcflag, "."))
@@ -126,7 +129,70 @@ func TestPGODevirtualize(t *testing.T) {
 	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")} {
+	for _, file := range []string{"devirt.go", "devirt_test.go", profFileName, 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)
+		}
+	}
+
+	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, profFileName)
+}
+
+// TestPGOPreprocessDevirtualize tests that specific functions are devirtualized when PGO
+// is applied to the exact source that was profiled. The input profile is PGO preprocessed file.
+func TestPGOPreprocessDevirtualize(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", preProfFileName, 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)
 		}
@@ -172,7 +238,7 @@ func TestPGODevirtualize(t *testing.T) {
 		//},
 	}
 
-	testPGODevirtualize(t, dir, want)
+	testPGODevirtualize(t, dir, want, preProfFileName)
 }
 
 // Regression test for https://go.dev/issue/65615. If a target function changes
@@ -190,7 +256,7 @@ func TestLookupFuncGeneric(t *testing.T) {
 	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")} {
+	for _, file := range []string{"devirt.go", "devirt_test.go", profFileName, 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)
 		}
@@ -238,7 +304,7 @@ func TestLookupFuncGeneric(t *testing.T) {
 		//},
 	}
 
-	testPGODevirtualize(t, dir, want)
+	testPGODevirtualize(t, dir, want, profFileName)
 }
 
 var multFnRe = regexp.MustCompile(`func MultFn\(a, b int64\) int64`)

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

@@ -18,6 +18,9 @@ import (
 	"testing"
 )
 
+const profFile = "inline_hot.pprof"
+const preProfFile = "inline_hot.pprof.node_map"
+
 func buildPGOInliningTest(t *testing.T, dir string, gcflag string) []byte {
 	const pkg = "example.com/pgo/inline"
 
@@ -43,12 +46,7 @@ go 1.19
 }
 
 // testPGOIntendedInlining tests that specific functions are inlined.
-func testPGOIntendedInlining(t *testing.T, dir string, preprocessed ...bool) {
-	defaultPGOPackValue := false
-	if len(preprocessed) > 0 {
-		defaultPGOPackValue = preprocessed[0]
-	}
-
+func testPGOIntendedInlining(t *testing.T, dir string, profFile string) {
 	testenv.MustHaveGoRun(t)
 	t.Parallel()
 
@@ -91,13 +89,7 @@ func testPGOIntendedInlining(t *testing.T, dir string, preprocessed ...bool) {
 
 	// Build the test with the profile. Use a smaller threshold to test.
 	// TODO: maybe adjust the test to work with default threshold.
-	var pprof string
-	if defaultPGOPackValue == false {
-		pprof = filepath.Join(dir, "inline_hot.pprof")
-	} else {
-		pprof = filepath.Join(dir, "inline_hot.pprof.node_map")
-	}
-	gcflag := fmt.Sprintf("-m -m -pgoprofile=%s -d=pgoinlinebudget=160,pgoinlinecdfthreshold=90", pprof)
+	gcflag := fmt.Sprintf("-m -m -pgoprofile=%s -d=pgoinlinebudget=160,pgoinlinecdfthreshold=90", profFile)
 	out := buildPGOInliningTest(t, dir, gcflag)
 
 	scanner := bufio.NewScanner(bytes.NewReader(out))
@@ -165,13 +157,13 @@ func TestPGOIntendedInlining(t *testing.T) {
 	// Copy the module to a scratch location so we can add a go.mod.
 	dir := t.TempDir()
 
-	for _, file := range []string{"inline_hot.go", "inline_hot_test.go", "inline_hot.pprof"} {
+	for _, file := range []string{"inline_hot.go", "inline_hot_test.go", profFile} {
 		if err := copyFile(filepath.Join(dir, file), filepath.Join(srcDir, file)); err != nil {
 			t.Fatalf("error copying %s: %v", file, err)
 		}
 	}
 
-	testPGOIntendedInlining(t, dir)
+	testPGOIntendedInlining(t, dir, profFile)
 }
 
 // TestPGOIntendedInlining tests that specific functions are inlined when PGO
@@ -186,13 +178,13 @@ func TestPGOPreprocessInlining(t *testing.T) {
 	// Copy the module to a scratch location so we can add a go.mod.
 	dir := t.TempDir()
 
-	for _, file := range []string{"inline_hot.go", "inline_hot_test.go", "inline_hot.pprof.node_map"} {
+	for _, file := range []string{"inline_hot.go", "inline_hot_test.go", preProfFile} {
 		if err := copyFile(filepath.Join(dir, file), filepath.Join(srcDir, file)); err != nil {
 			t.Fatalf("error copying %s: %v", file, err)
 		}
 	}
 
-	testPGOIntendedInlining(t, dir, true)
+	testPGOIntendedInlining(t, dir, preProfFile)
 }
 
 // TestPGOIntendedInlining tests that specific functions are inlined when PGO
@@ -208,7 +200,7 @@ func TestPGOIntendedInliningShiftedLines(t *testing.T) {
 	dir := t.TempDir()
 
 	// Copy most of the files unmodified.
-	for _, file := range []string{"inline_hot_test.go", "inline_hot.pprof"} {
+	for _, file := range []string{"inline_hot_test.go", profFile} {
 		if err := copyFile(filepath.Join(dir, file), filepath.Join(srcDir, file)); err != nil {
 			t.Fatalf("error copying %s : %v", file, err)
 		}
@@ -240,7 +232,7 @@ func TestPGOIntendedInliningShiftedLines(t *testing.T) {
 
 	dst.Close()
 
-	testPGOIntendedInlining(t, dir)
+	testPGOIntendedInlining(t, dir, profFile)
 }
 
 // TestPGOSingleIndex tests that the sample index can not be 1 and compilation
@@ -270,15 +262,15 @@ func TestPGOSingleIndex(t *testing.T) {
 			// Copy the module to a scratch location so we can add a go.mod.
 			dir := t.TempDir()
 
-			originalPprofFile, err := os.Open(filepath.Join(srcDir, "inline_hot.pprof"))
+			originalPprofFile, err := os.Open(filepath.Join(srcDir, profFile))
 			if err != nil {
-				t.Fatalf("error opening inline_hot.pprof: %v", err)
+				t.Fatalf("error opening %v: %v", profFile, err)
 			}
 			defer originalPprofFile.Close()
 
 			p, err := profile.Parse(originalPprofFile)
 			if err != nil {
-				t.Fatalf("error parsing inline_hot.pprof: %v", err)
+				t.Fatalf("error parsing %v: %v", profFile, err)
 			}
 
 			// Move the samples count value-type to the 0 index.
@@ -289,14 +281,14 @@ func TestPGOSingleIndex(t *testing.T) {
 				s.Value = []int64{s.Value[tc.originalIndex]}
 			}
 
-			modifiedPprofFile, err := os.Create(filepath.Join(dir, "inline_hot.pprof"))
+			modifiedPprofFile, err := os.Create(filepath.Join(dir, profFile))
 			if err != nil {
-				t.Fatalf("error creating inline_hot.pprof: %v", err)
+				t.Fatalf("error creating %v: %v", profFile, err)
 			}
 			defer modifiedPprofFile.Close()
 
 			if err := p.Write(modifiedPprofFile); err != nil {
-				t.Fatalf("error writing inline_hot.pprof: %v", err)
+				t.Fatalf("error writing %v: %v", profFile, err)
 			}
 
 			for _, file := range []string{"inline_hot.go", "inline_hot_test.go"} {
@@ -305,7 +297,7 @@ func TestPGOSingleIndex(t *testing.T) {
 				}
 			}
 
-			testPGOIntendedInlining(t, dir)
+			testPGOIntendedInlining(t, dir, profFile)
 		})
 	}
 }
@@ -343,13 +335,13 @@ func TestPGOHash(t *testing.T) {
 	// Copy the module to a scratch location so we can add a go.mod.
 	dir := t.TempDir()
 
-	for _, file := range []string{"inline_hot.go", "inline_hot_test.go", "inline_hot.pprof"} {
+	for _, file := range []string{"inline_hot.go", "inline_hot_test.go", profFile} {
 		if err := copyFile(filepath.Join(dir, file), filepath.Join(srcDir, file)); err != nil {
 			t.Fatalf("error copying %s: %v", file, err)
 		}
 	}
 
-	pprof := filepath.Join(dir, "inline_hot.pprof")
+	pprof := filepath.Join(dir, profFile)
 	// build with -trimpath so the source location (thus the hash)
 	// does not depend on the temporary directory path.
 	gcflag0 := fmt.Sprintf("-pgoprofile=%s -trimpath %s=>%s -d=pgoinlinebudget=160,pgoinlinecdfthreshold=90,pgodebug=1", pprof, dir, pkg)

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

@@ -0,0 +1,52 @@
+GO PREPROFILE V1
+example.com/pgo/devirtualize.ExerciseFuncClosure
+example.com/pgo/devirtualize/mult%2epkg.MultClosure.func1
+18 93
+example.com/pgo/devirtualize.ExerciseIface
+example.com/pgo/devirtualize/mult%2epkg.NegMult.Multiply
+49 4
+example.com/pgo/devirtualize.ExerciseFuncConcrete
+example.com/pgo/devirtualize.AddFn
+48 103
+example.com/pgo/devirtualize.ExerciseFuncField
+example.com/pgo/devirtualize/mult%2epkg.NegMultFn
+23 8
+example.com/pgo/devirtualize.ExerciseFuncField
+example.com/pgo/devirtualize/mult%2epkg.MultFn
+23 94
+example.com/pgo/devirtualize.ExerciseIface
+example.com/pgo/devirtualize/mult%2epkg.Mult.Multiply
+49 40
+example.com/pgo/devirtualize.ExerciseIface
+example.com/pgo/devirtualize.Add.Add
+49 55
+example.com/pgo/devirtualize.ExerciseFuncConcrete
+example.com/pgo/devirtualize/mult%2epkg.NegMultFn
+48 8
+example.com/pgo/devirtualize.ExerciseFuncClosure
+example.com/pgo/devirtualize/mult%2epkg.NegMultClosure.func1
+18 10
+example.com/pgo/devirtualize.ExerciseIface
+example.com/pgo/devirtualize.Sub.Add
+49 7
+example.com/pgo/devirtualize.ExerciseFuncField
+example.com/pgo/devirtualize.AddFn
+23 101
+example.com/pgo/devirtualize.ExerciseFuncField
+example.com/pgo/devirtualize.SubFn
+23 12
+example.com/pgo/devirtualize.BenchmarkDevirtFuncConcrete
+example.com/pgo/devirtualize.ExerciseFuncConcrete
+1 2
+example.com/pgo/devirtualize.ExerciseFuncConcrete
+example.com/pgo/devirtualize/mult%2epkg.MultFn
+48 91
+example.com/pgo/devirtualize.ExerciseFuncConcrete
+example.com/pgo/devirtualize.SubFn
+48 5
+example.com/pgo/devirtualize.ExerciseFuncClosure
+example.com/pgo/devirtualize.Add.Add
+18 92
+example.com/pgo/devirtualize.ExerciseFuncClosure
+example.com/pgo/devirtualize.Sub.Add
+18 14

src/cmd/compile/internal/test/testdata/pgo/inline/inline_hot.pprof.node_map

@@ -1,13 +1,13 @@
 GO PREPROFILE V1
 example.com/pgo/inline.benchmarkB
 example.com/pgo/inline.A
-18 17 0 1 1
+18 1
 example.com/pgo/inline.(*BS).NS
 example.com/pgo/inline.T
-13 53 124 129 2
+8 3
 example.com/pgo/inline.(*BS).NS
 example.com/pgo/inline.T
-8 53 124 129 3
+13 2
 example.com/pgo/inline.A
 example.com/pgo/inline.(*BS).NS
-7 74 1 130 129
+7 129