cmd/compile: fix transitive inlining of generic functions

If an imported, non-generic function F transitively calls a generic
function G[T], we may need to call CanInline on G[T].

While here, we can also take advantage of the fact that we know G[T]
was already seen and compiled in an imported package, so we don't need
to call InlineCalls or add it to typecheck.Target.Decls. This saves us
from wasting compile time re-creating DUPOK symbols that we know
already exist in the imported package's link objects.

Fixes #56280.

Change-Id: I3336786bee01616ee9f2b18908738e4ca41c8102
Reviewed-on: https://go-review.googlesource.com/c/go/+/443535
Run-TryBot: Matthew Dempsky <[email protected]>
TryBot-Result: Gopher Robot <[email protected]>
Reviewed-by: David Chase <[email protected]>
Auto-Submit: Matthew Dempsky <[email protected]>

Author: mdempsky

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

@@ -83,7 +83,7 @@ var (
 )
 
 // pgoInlinePrologue records the hot callsites from ir-graph.
-func pgoInlinePrologue(p *pgo.Profile) {
+func pgoInlinePrologue(p *pgo.Profile, decls []ir.Node) {
 	if s, err := strconv.ParseFloat(base.Debug.InlineHotCallSiteCDFThreshold, 64); err == nil {
 		inlineCDFHotCallSiteThresholdPercent = s
 	}
@@ -104,7 +104,7 @@ func pgoInlinePrologue(p *pgo.Profile) {
 		}
 	}
 	// mark hot call sites
-	ir.VisitFuncsBottomUp(typecheck.Target.Decls, func(list []*ir.Func, recursive bool) {
+	ir.VisitFuncsBottomUp(decls, func(list []*ir.Func, recursive bool) {
 		for _, f := range list {
 			name := ir.PkgFuncName(f)
 			if n, ok := p.WeightedCG.IRNodes[name]; ok {
@@ -164,9 +164,9 @@ func computeThresholdFromCDF(p *pgo.Profile) (float64, []pgo.NodeMapKey) {
 }
 
 // pgoInlineEpilogue updates IRGraph after inlining.
-func pgoInlineEpilogue(p *pgo.Profile) {
+func pgoInlineEpilogue(p *pgo.Profile, decls []ir.Node) {
 	if base.Debug.PGOInline >= 2 {
-		ir.VisitFuncsBottomUp(typecheck.Target.Decls, func(list []*ir.Func, recursive bool) {
+		ir.VisitFuncsBottomUp(decls, func(list []*ir.Func, recursive bool) {
 			for _, f := range list {
 				name := ir.PkgFuncName(f)
 				if n, ok := p.WeightedCG.IRNodes[name]; ok {
@@ -182,11 +182,16 @@ func pgoInlineEpilogue(p *pgo.Profile) {
 
 // InlinePackage finds functions that can be inlined and clones them before walk expands them.
 func InlinePackage(p *pgo.Profile) {
+	InlineDecls(p, typecheck.Target.Decls, true)
+}
+
+// InlineDecls applies inlining to the given batch of declarations.
+func InlineDecls(p *pgo.Profile, decls []ir.Node, doInline bool) {
 	if p != nil {
-		pgoInlinePrologue(p)
+		pgoInlinePrologue(p, decls)
 	}
 
-	ir.VisitFuncsBottomUp(typecheck.Target.Decls, func(list []*ir.Func, recursive bool) {
+	ir.VisitFuncsBottomUp(decls, func(list []*ir.Func, recursive bool) {
 		numfns := numNonClosures(list)
 		for _, n := range list {
 			if !recursive || numfns > 1 {
@@ -199,12 +204,14 @@ func InlinePackage(p *pgo.Profile) {
 					fmt.Printf("%v: cannot inline %v: recursive\n", ir.Line(n), n.Nname)
 				}
 			}
-			InlineCalls(n, p)
+			if doInline {
+				InlineCalls(n, p)
+			}
 		}
 	})
 
 	if p != nil {
-		pgoInlineEpilogue(p)
+		pgoInlineEpilogue(p, decls)
 	}
 }
 

src/cmd/compile/internal/noder/reader.go

@@ -3485,7 +3485,7 @@ func unifiedInlineCall(call *ir.CallExpr, fn *ir.Func, inlIndex int) *ir.Inlined
 		// potentially be recursively inlined themselves; but we shouldn't
 		// need to read in the non-inlined bodies for the declarations
 		// themselves. But currently it's an easy fix to #50552.
-		readBodies(typecheck.Target)
+		readBodies(typecheck.Target, true)
 
 		deadcode.Func(r.curfn)
 

src/cmd/compile/internal/noder/unified.go

@@ -101,7 +101,7 @@ func unified(noders []*noder) {
 		}
 	}
 
-	readBodies(target)
+	readBodies(target, false)
 
 	// Check that nothing snuck past typechecking.
 	for _, n := range target.Decls {
@@ -123,7 +123,13 @@ func unified(noders []*noder) {
 
 // readBodies iteratively expands all pending dictionaries and
 // function bodies.
-func readBodies(target *ir.Package) {
+//
+// If duringInlining is true, then the inline.InlineDecls is called as
+// necessary on instantiations of imported generic functions, so their
+// inlining costs can be computed.
+func readBodies(target *ir.Package, duringInlining bool) {
+	var inlDecls []ir.Node
+
 	// Don't use range--bodyIdx can add closures to todoBodies.
 	for {
 		// The order we expand dictionaries and bodies doesn't matter, so
@@ -152,7 +158,11 @@ func readBodies(target *ir.Package) {
 			// Instantiated generic function: add to Decls for typechecking
 			// and compilation.
 			if fn.OClosure == nil && len(pri.dict.targs) != 0 {
-				target.Decls = append(target.Decls, fn)
+				if duringInlining {
+					inlDecls = append(inlDecls, fn)
+				} else {
+					target.Decls = append(target.Decls, fn)
+				}
 			}
 
 			continue
@@ -163,6 +173,30 @@ func readBodies(target *ir.Package) {
 
 	todoDicts = nil
 	todoBodies = nil
+
+	if len(inlDecls) != 0 {
+		// If we instantiated any generic functions during inlining, we need
+		// to call CanInline on them so they'll be transitively inlined
+		// correctly (#56280).
+		//
+		// We know these functions were already compiled in an imported
+		// package though, so we don't need to actually apply InlineCalls or
+		// save the function bodies any further than this.
+		//
+		// We can also lower the -m flag to 0, to suppress duplicate "can
+		// inline" diagnostics reported against the imported package. Again,
+		// we already reported those diagnostics in the original package, so
+		// it's pointless repeating them here.
+
+		oldLowerM := base.Flag.LowerM
+		base.Flag.LowerM = 0
+		inline.InlineDecls(nil, inlDecls, false)
+		base.Flag.LowerM = oldLowerM
+
+		for _, fn := range inlDecls {
+			fn.(*ir.Func).Body = nil // free memory
+		}
+	}
 }
 
 // writePkgStub type checks the given parsed source files,

test/fixedbugs/issue56280.dir/a.go

@@ -0,0 +1,11 @@
+// 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.
+
+package a
+
+func F() { // ERROR "can inline F"
+	g(0) // ERROR "inlining call to g\[go.shape.int\]"
+}
+
+func g[T any](_ T) {} // ERROR "can inline g\[int\]" "can inline g\[go.shape.int\]" "inlining call to g\[go.shape.int\]"

test/fixedbugs/issue56280.dir/main.go

@@ -0,0 +1,11 @@
+// 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.
+
+package main
+
+import "test/a"
+
+func main() { // ERROR "can inline main"
+	a.F() // ERROR "inlining call to a.F" "inlining call to a.g\[go.shape.int\]"
+}

test/fixedbugs/issue56280.go

@@ -0,0 +1,7 @@
+// errorcheckdir -0 -m
+
+// 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.
+
+package ignored

test/run.go

@@ -2035,6 +2035,7 @@ var types2Failures32Bit = setOf(
 
 var go118Failures = setOf(
 	"fixedbugs/issue54343.go",  // 1.18 compiler assigns receiver parameter to global variable
+	"fixedbugs/issue56280.go",  // 1.18 compiler doesn't support inlining generic functions
 	"typeparam/nested.go",      // 1.18 compiler doesn't support function-local types with generics
 	"typeparam/issue47631.go",  // 1.18 can not handle local type declarations
 	"typeparam/issue51521.go",  // 1.18 compiler produces bad panic message and link error