From: Matthew Dempsky Date: Mon, 17 Oct 2022 23:57:07 +0000 (-0700) Subject: cmd/compile: fix transitive inlining of generic functions X-Git-Tag: go1.20rc1~371 X-Git-Url: http://www.git.cypherpunks.su/?a=commitdiff_plain;h=9944ba757b0f8005cfb7715d41592c7e13c0a2b8;p=gostls13.git 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 TryBot-Result: Gopher Robot Reviewed-by: David Chase Auto-Submit: Matthew Dempsky --- diff --git a/src/cmd/compile/internal/inline/inl.go b/src/cmd/compile/internal/inline/inl.go index 6aaecfc7c6..aebe32869a 100644 --- a/src/cmd/compile/internal/inline/inl.go +++ b/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) } } diff --git a/src/cmd/compile/internal/noder/reader.go b/src/cmd/compile/internal/noder/reader.go index fe90f52b4d..d03da27a46 100644 --- a/src/cmd/compile/internal/noder/reader.go +++ b/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) diff --git a/src/cmd/compile/internal/noder/unified.go b/src/cmd/compile/internal/noder/unified.go index b8e4fe78d7..61767ea2d9 100644 --- a/src/cmd/compile/internal/noder/unified.go +++ b/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, diff --git a/test/fixedbugs/issue56280.dir/a.go b/test/fixedbugs/issue56280.dir/a.go new file mode 100644 index 0000000000..289b06a7b3 --- /dev/null +++ b/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\]" diff --git a/test/fixedbugs/issue56280.dir/main.go b/test/fixedbugs/issue56280.dir/main.go new file mode 100644 index 0000000000..06092b17f0 --- /dev/null +++ b/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\]" +} diff --git a/test/fixedbugs/issue56280.go b/test/fixedbugs/issue56280.go new file mode 100644 index 0000000000..1afbe9ebf7 --- /dev/null +++ b/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 diff --git a/test/run.go b/test/run.go index 167eeac689..d0178b57c7 100644 --- a/test/run.go +++ b/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