Revert "[release-branch.go1.19] cmd/compile: defer transitive inlining until after AST is edited"

This reverts commit 837b1314fde57c0afad96bbfa050b47ce304c204.

Reason for revert: The branch is currently frozen for the release.

Change-Id: I800e241b8676564ea893ae673d407b41e55f0473
Reviewed-on: https://go-review.googlesource.com/c/go/+/481796
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Dmitri Shuralyov <dmitshur@golang.org>
Run-TryBot: Michael Knyszek <mknyszek@google.com>
Auto-Submit: Michael Knyszek <mknyszek@google.com>
Reviewed-by: David Chase <drchase@google.com>

2 files changed, 30 insertions, 65 deletions

diff --git a/src/cmd/compile/internal/inline/inl.go b/src/cmd/compile/internal/inline/inl.go
index 5fc49c11a8..16b6a62bba 100644
--- a/src/cmd/compile/internal/inline/inl.go
+++ b/src/cmd/compile/internal/inline/inl.go
@@ -502,23 +502,18 @@ func InlineCalls(fn *ir.Func) {
 	if isBigFunc(fn) {
 		maxCost = inlineBigFunctionMaxCost
 	}
-	var inlCalls []*ir.InlinedCallExpr
+	// Map to keep track of functions that have been inlined at a particular
+	// call site, in order to stop inlining when we reach the beginning of a
+	// recursion cycle again. We don't inline immediately recursive functions,
+	// but allow inlining if there is a recursion cycle of many functions.
+	// Most likely, the inlining will stop before we even hit the beginning of
+	// the cycle again, but the map catches the unusual case.
+	inlMap := make(map[*ir.Func]bool)
 	var edit func(ir.Node) ir.Node
 	edit = func(n ir.Node) ir.Node {
-		return inlnode(n, maxCost, &inlCalls, edit)
+		return inlnode(n, maxCost, inlMap, edit)
 	}
 	ir.EditChildren(fn, edit)
-
-	// If we inlined any calls, we want to recursively visit their
-	// bodies for further inlining. However, we need to wait until
-	// *after* the original function body has been expanded, or else
-	// inlCallee can have false positives (e.g., #54632).
-	for len(inlCalls) > 0 {
-		call := inlCalls[0]
-		inlCalls = inlCalls[1:]
-		ir.EditChildren(call, edit)
-	}
-
 	ir.CurFunc = savefn
 }
 
@@ -536,7 +531,7 @@ func InlineCalls(fn *ir.Func) {
 // The result of inlnode MUST be assigned back to n, e.g.
 //
 //	n.Left = inlnode(n.Left)
-func inlnode(n ir.Node, maxCost int32, inlCalls *[]*ir.InlinedCallExpr, edit func(ir.Node) ir.Node) ir.Node {
+func inlnode(n ir.Node, maxCost int32, inlMap map[*ir.Func]bool, edit func(ir.Node) ir.Node) ir.Node {
 	if n == nil {
 		return n
 	}
@@ -598,7 +593,7 @@ func inlnode(n ir.Node, maxCost int32, inlCalls *[]*ir.InlinedCallExpr, edit fun
 			break
 		}
 		if fn := inlCallee(call.X); fn != nil && typecheck.HaveInlineBody(fn) {
-			n = mkinlcall(call, fn, maxCost, inlCalls, edit)
+			n = mkinlcall(call, fn, maxCost, inlMap, edit)
 		}
 	}
 
@@ -672,7 +667,7 @@ var NewInline = func(call *ir.CallExpr, fn *ir.Func, inlIndex int) *ir.InlinedCa
 // The result of mkinlcall MUST be assigned back to n, e.g.
 //
 //	n.Left = mkinlcall(n.Left, fn, isddd)
-func mkinlcall(n *ir.CallExpr, fn *ir.Func, maxCost int32, inlCalls *[]*ir.InlinedCallExpr, edit func(ir.Node) ir.Node) ir.Node {
+func mkinlcall(n *ir.CallExpr, fn *ir.Func, maxCost int32, inlMap map[*ir.Func]bool, edit func(ir.Node) ir.Node) ir.Node {
 	if fn.Inl == nil {
 		if logopt.Enabled() {
 			logopt.LogOpt(n.Pos(), "cannotInlineCall", "inline", ir.FuncName(ir.CurFunc),
@@ -748,27 +743,22 @@ func mkinlcall(n *ir.CallExpr, fn *ir.Func, maxCost int32, inlCalls *[]*ir.Inlin
 		return n
 	}
 
-	parent := base.Ctxt.PosTable.Pos(n.Pos()).Base().InliningIndex()
-	sym := fn.Linksym()
-
-	// Check if we've already inlined this function at this particular
-	// call site, in order to stop inlining when we reach the beginning
-	// of a recursion cycle again. We don't inline immediately recursive
-	// functions, but allow inlining if there is a recursion cycle of
-	// many functions. Most likely, the inlining will stop before we
-	// even hit the beginning of the cycle again, but this catches the
-	// unusual case.
-	for inlIndex := parent; inlIndex >= 0; inlIndex = base.Ctxt.InlTree.Parent(inlIndex) {
-		if base.Ctxt.InlTree.InlinedFunction(inlIndex) == sym {
-			if base.Flag.LowerM > 1 {
-				fmt.Printf("%v: cannot inline %v into %v: repeated recursive cycle\n", ir.Line(n), fn, ir.FuncName(ir.CurFunc))
-			}
-			return n
+	if inlMap[fn] {
+		if base.Flag.LowerM > 1 {
+			fmt.Printf("%v: cannot inline %v into %v: repeated recursive cycle\n", ir.Line(n), fn, ir.FuncName(ir.CurFunc))
 		}
+		return n
 	}
+	inlMap[fn] = true
+	defer func() {
+		inlMap[fn] = false
+	}()
 
 	typecheck.FixVariadicCall(n)
 
+	parent := base.Ctxt.PosTable.Pos(n.Pos()).Base().InliningIndex()
+
+	sym := fn.Linksym()
 	inlIndex := base.Ctxt.InlTree.Add(parent, n.Pos(), sym)
 
 	if base.Flag.GenDwarfInl > 0 {
@@ -790,12 +780,18 @@ func mkinlcall(n *ir.CallExpr, fn *ir.Func, maxCost int32, inlCalls *[]*ir.Inlin
 		res = oldInline(n, fn, inlIndex)
 	}
 
+	// transitive inlining
+	// might be nice to do this before exporting the body,
+	// but can't emit the body with inlining expanded.
+	// instead we emit the things that the body needs
+	// and each use must redo the inlining.
+	// luckily these are small.
+	ir.EditChildren(res, edit)
+
 	if base.Flag.LowerM > 2 {
 		fmt.Printf("%v: After inlining %+v\n\n", ir.Line(res), res)
 	}
 
-	*inlCalls = append(*inlCalls, res)
-
 	return res
 }
 
diff --git a/test/fixedbugs/issue54632.go b/test/fixedbugs/issue54632.go
deleted file mode 100644
index 0d4e32f28f..0000000000
--- a/test/fixedbugs/issue54632.go
+++ /dev/null
@@ -1,31 +0,0 @@
-// run
-
-// Copyright 2022 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.
-
-// The inliner would erroneously scan the caller function's body for
-// reassignments *before* substituting the inlined function call body,
-// which could cause false positives in deciding when it's safe to
-// transitively inline indirect function calls.
-
-package main
-
-func main() {
-	bug1()
-	bug2(fail)
-}
-
-func bug1() {
-	fn := fail
-	fn = pass
-	fn()
-}
-
-func bug2(fn func()) {
-	fn = pass
-	fn()
-}
-
-func pass() {}
-func fail() { panic("FAIL") }