From f56c29331951c9d1a48f5b7627f4bb98e7eeb80f Mon Sep 17 00:00:00 2001 From: Than McIntosh Date: Fri, 15 Sep 2023 14:46:03 -0400 Subject: [PATCH] cmd/compile/internal/inline: tweak "returns inlinable func" heuristic The code that analyzes function return values checks for cases where a function F always returns the same inlinable function, e.g. func returnsFunc() func(*int, int) { return setit } func setit(p *int, v int) { *p = v } The check for inlinability was being done by looking at "fn.Inl != nil", which is probably not what we want, since it includes functions whose cost value is between 80 and 160 and may only be inlined if lots of other heuristics kick in. This patch changes the "always returns same inlinable func" heuristic to ensure that the func in question has a size of 80 or less, so as to restrict this case to functions that have a high likelihood of being inlined. Change-Id: I06003bca1c56c401df8fd51c922a59c61aa86bea Reviewed-on: https://go-review.googlesource.com/c/go/+/529116 LUCI-TryBot-Result: Go LUCI Reviewed-by: Matthew Dempsky --- src/cmd/compile/internal/inline/inl.go | 8 ++-- .../internal/inline/inlheur/analyze.go | 39 ++++++++++--------- .../inline/inlheur/analyze_func_returns.go | 27 ++++++++----- 3 files changed, 42 insertions(+), 32 deletions(-) diff --git a/src/cmd/compile/internal/inline/inl.go b/src/cmd/compile/internal/inline/inl.go index 6765e199d0..992ae632e2 100644 --- a/src/cmd/compile/internal/inline/inl.go +++ b/src/cmd/compile/internal/inline/inl.go @@ -149,7 +149,7 @@ func InlinePackage(p *pgo.Profile) { garbageCollectUnreferencedHiddenClosures() if base.Debug.DumpInlFuncProps != "" { - inlheur.DumpFuncProps(nil, base.Debug.DumpInlFuncProps, nil) + inlheur.DumpFuncProps(nil, base.Debug.DumpInlFuncProps, nil, inlineMaxBudget) } if goexperiment.NewInliner { postProcessCallSites(p) @@ -283,8 +283,8 @@ func CanInline(fn *ir.Func, profile *pgo.Profile) { var funcProps *inlheur.FuncProps if goexperiment.NewInliner || inlheur.UnitTesting() { - funcProps = inlheur.AnalyzeFunc(fn, - func(fn *ir.Func) { CanInline(fn, profile) }) + callCanInline := func(fn *ir.Func) { CanInline(fn, profile) } + funcProps = inlheur.AnalyzeFunc(fn, callCanInline, inlineMaxBudget) } var reason string // reason, if any, that the function was not inlined @@ -802,7 +802,7 @@ func InlineCalls(fn *ir.Func, profile *pgo.Profile) { } if base.Debug.DumpInlFuncProps != "" && !fn.Wrapper() { inlheur.DumpFuncProps(fn, base.Debug.DumpInlFuncProps, - func(fn *ir.Func) { CanInline(fn, profile) }) + func(fn *ir.Func) { CanInline(fn, profile) }, inlineMaxBudget) } savefn := ir.CurFunc ir.CurFunc = fn diff --git a/src/cmd/compile/internal/inline/inlheur/analyze.go b/src/cmd/compile/internal/inline/inlheur/analyze.go index 3348e08975..8e54c9f123 100644 --- a/src/cmd/compile/internal/inline/inlheur/analyze.go +++ b/src/cmd/compile/internal/inline/inlheur/analyze.go @@ -51,27 +51,29 @@ type propAnalyzer interface { // parsing a dump. This is the reason why we have file/fname/line // fields below instead of just an *ir.Func field. type fnInlHeur struct { - fname string - file string - line uint - props *FuncProps - cstab CallSiteTab + fname string + file string + line uint + inlineMaxBudget int32 + props *FuncProps + cstab CallSiteTab } var fpmap = map[*ir.Func]fnInlHeur{} -func AnalyzeFunc(fn *ir.Func, canInline func(*ir.Func)) *FuncProps { +func AnalyzeFunc(fn *ir.Func, canInline func(*ir.Func), inlineMaxBudget int32) *FuncProps { if fih, ok := fpmap[fn]; ok { return fih.props } - fp, fcstab := computeFuncProps(fn, canInline) + fp, fcstab := computeFuncProps(fn, canInline, inlineMaxBudget) file, line := fnFileLine(fn) entry := fnInlHeur{ - fname: fn.Sym().Name, - file: file, - line: line, - props: fp, - cstab: fcstab, + fname: fn.Sym().Name, + file: file, + line: line, + inlineMaxBudget: inlineMaxBudget, + props: fp, + cstab: fcstab, } // Merge this functions call sites into the package level table. if err := cstab.merge(fcstab); err != nil { @@ -88,13 +90,13 @@ func AnalyzeFunc(fn *ir.Func, canInline func(*ir.Func)) *FuncProps { // computeFuncProps examines the Go function 'fn' and computes for it // a function "properties" object, to be used to drive inlining // heuristics. See comments on the FuncProps type for more info. -func computeFuncProps(fn *ir.Func, canInline func(*ir.Func)) (*FuncProps, CallSiteTab) { +func computeFuncProps(fn *ir.Func, canInline func(*ir.Func), inlineMaxBudget int32) (*FuncProps, CallSiteTab) { enableDebugTraceIfEnv() if debugTrace&debugTraceFuncs != 0 { fmt.Fprintf(os.Stderr, "=-= starting analysis of func %v:\n%+v\n", fn.Sym().Name, fn) } - ra := makeResultsAnalyzer(fn, canInline) + ra := makeResultsAnalyzer(fn, canInline, inlineMaxBudget) pa := makeParamsAnalyzer(fn) ffa := makeFuncFlagsAnalyzer(fn) analyzers := []propAnalyzer{ffa, ra, pa} @@ -149,16 +151,15 @@ func UnitTesting() bool { // cached set of properties to the file given in 'dumpfile'. Used for // the "-d=dumpinlfuncprops=..." command line flag, intended for use // primarily in unit testing. -func DumpFuncProps(fn *ir.Func, dumpfile string, canInline func(*ir.Func)) { +func DumpFuncProps(fn *ir.Func, dumpfile string, canInline func(*ir.Func), inlineMaxBudget int32) { if fn != nil { enableDebugTraceIfEnv() dmp := func(fn *ir.Func) { if !goexperiment.NewInliner { ScoreCalls(fn) } - captureFuncDumpEntry(fn, canInline) + captureFuncDumpEntry(fn, canInline, inlineMaxBudget) } - captureFuncDumpEntry(fn, canInline) dmp(fn) ir.Visit(fn, func(n ir.Node) { if clo, ok := n.(*ir.ClosureExpr); ok { @@ -221,7 +222,7 @@ func emitDumpToFile(dumpfile string) { // and enqueues it for later dumping. Used for the // "-d=dumpinlfuncprops=..." command line flag, intended for use // primarily in unit testing. -func captureFuncDumpEntry(fn *ir.Func, canInline func(*ir.Func)) { +func captureFuncDumpEntry(fn *ir.Func, canInline func(*ir.Func), inlineMaxBudget int32) { // avoid capturing compiler-generated equality funcs. if strings.HasPrefix(fn.Sym().Name, ".eq.") { return @@ -230,7 +231,7 @@ func captureFuncDumpEntry(fn *ir.Func, canInline func(*ir.Func)) { // Props object should already be present, unless this is a // directly recursive routine. if !ok { - AnalyzeFunc(fn, canInline) + AnalyzeFunc(fn, canInline, inlineMaxBudget) fih = fpmap[fn] if fn.Inl != nil && fn.Inl.Properties == "" { fn.Inl.Properties = fih.props.SerializeToString() diff --git a/src/cmd/compile/internal/inline/inlheur/analyze_func_returns.go b/src/cmd/compile/internal/inline/inlheur/analyze_func_returns.go index e015961474..8107143631 100644 --- a/src/cmd/compile/internal/inline/inlheur/analyze_func_returns.go +++ b/src/cmd/compile/internal/inline/inlheur/analyze_func_returns.go @@ -16,10 +16,11 @@ import ( // computing flags/properties for the return values of a specific Go // function, as part of inline heuristics synthesis. type returnsAnalyzer struct { - fname string - props []ResultPropBits - values []resultVal - canInline func(*ir.Func) + fname string + props []ResultPropBits + values []resultVal + canInline func(*ir.Func) + inlineMaxBudget int32 } // resultVal captures information about a specific result returned from @@ -35,7 +36,7 @@ type resultVal struct { derived bool // see deriveReturnFlagsFromCallee below } -func makeResultsAnalyzer(fn *ir.Func, canInline func(*ir.Func)) *returnsAnalyzer { +func makeResultsAnalyzer(fn *ir.Func, canInline func(*ir.Func), inlineMaxBudget int32) *returnsAnalyzer { results := fn.Type().Results() props := make([]ResultPropBits, len(results)) vals := make([]resultVal, len(results)) @@ -52,9 +53,10 @@ func makeResultsAnalyzer(fn *ir.Func, canInline func(*ir.Func)) *returnsAnalyzer vals[i].top = true } return &returnsAnalyzer{ - props: props, - values: vals, - canInline: canInline, + props: props, + values: vals, + canInline: canInline, + inlineMaxBudget: inlineMaxBudget, } } @@ -74,7 +76,14 @@ func (ra *returnsAnalyzer) setResults(fp *FuncProps) { ra.canInline(f) } } - if f.Inl != nil { + // HACK: in order to allow for call site score + // adjustments, we used a relaxed inline budget in + // determining inlinability. Here what we want to know is + // whether the func in question is likely to be inlined, + // as opposed to whether it might possibly be inlined if + // all the right score adjustments happened, so check the + // cost here as well. + if f.Inl != nil && f.Inl.Cost <= ra.inlineMaxBudget { ra.props[i] = ResultAlwaysSameInlinableFunc } } -- 2.44.0