From 9e90a15ba4f6ad7d3a61ecf81bf00abb386fbe0d Mon Sep 17 00:00:00 2001 From: Than McIntosh Date: Fri, 15 Sep 2023 15:06:06 -0400 Subject: [PATCH] cmd/compile/internal/inline/inlheur: enhance call result scoring This patch makes a small enhancement to call result scoring, to make it more independent of param value heuristics. For this pair of functions: func caller() { v := callee(10) <<-- this callsite if v > 101 { ... } } func callee(x int) { if x < 0 { G = 1 } return 9 } The score for the specified call site above would be adjusted only once, for the "pass constant to parameter that feeds 'if' statement" heuristic, which didn't reflect the fact that doing the inline enables not one but two specific deadcode opportunities (first for the code inside the inlined routine body, then for the "if" downstream of the inlined call). This patch changes the call result scoring machinery to use a separate set of mask bits, so that we can more accurately handle the case above. Change-Id: I700166d0c990c037215b9f904e9984886986c600 Reviewed-on: https://go-review.googlesource.com/c/go/+/529117 LUCI-TryBot-Result: Go LUCI Reviewed-by: Matthew Dempsky --- .../inline/inlheur/score_callresult_uses.go | 22 ++----- .../inline/inlheur/scoreadjusttyp_string.go | 38 ++++++------ .../internal/inline/inlheur/scoring.go | 60 ++++++++++++++----- .../inline/inlheur/testdata/props/returns2.go | 29 +++++---- 4 files changed, 89 insertions(+), 60 deletions(-) diff --git a/src/cmd/compile/internal/inline/inlheur/score_callresult_uses.go b/src/cmd/compile/internal/inline/inlheur/score_callresult_uses.go index 76e250a33b..d13e1c3286 100644 --- a/src/cmd/compile/internal/inline/inlheur/score_callresult_uses.go +++ b/src/cmd/compile/internal/inline/inlheur/score_callresult_uses.go @@ -267,10 +267,8 @@ func (rua *resultUseAnalyzer) callTargetCheckResults(call ir.Node) { rua.fn.Sym().Name, rname) } if cs := rua.returnHasProp(rname, ResultIsConcreteTypeConvertedToInterface); cs != nil { - // FIXME: add cond level support here - adj := passConcreteToItfCallAdj - cs.Score, cs.ScoreMask = adjustScore(adj, cs.Score, cs.ScoreMask) - adj = callResultRescoreAdj + + adj := returnFeedsConcreteToInterfaceCallAdj cs.Score, cs.ScoreMask = adjustScore(adj, cs.Score, cs.ScoreMask) } case ir.OCALLFUNC: @@ -285,17 +283,12 @@ func (rua *resultUseAnalyzer) callTargetCheckResults(call ir.Node) { } } if cs := rua.returnHasProp(rname, ResultAlwaysSameInlinableFunc); cs != nil { - // FIXME: add cond level support here - adj := passInlinableFuncToIndCallAdj - cs.Score, cs.ScoreMask = adjustScore(adj, cs.Score, cs.ScoreMask) - adj = callResultRescoreAdj + adj := returnFeedsInlinableFuncToIndCallAdj cs.Score, cs.ScoreMask = adjustScore(adj, cs.Score, cs.ScoreMask) } else if cs := rua.returnHasProp(rname, ResultAlwaysSameFunc); cs != nil { - // FIXME: add cond level support here - adj := passFuncToIndCallAdj - cs.Score, cs.ScoreMask = adjustScore(adj, cs.Score, cs.ScoreMask) - adj = callResultRescoreAdj + adj := returnFeedsFuncToIndCallAdj cs.Score, cs.ScoreMask = adjustScore(adj, cs.Score, cs.ScoreMask) + } } } @@ -351,10 +344,7 @@ func (rua *resultUseAnalyzer) foldCheckResults(cond ir.Node) { if !ShouldFoldIfNameConstant(cond, namesUsed) { return } - // FIXME: add cond level support here - adj := passConstToIfAdj - cs.Score, cs.ScoreMask = adjustScore(adj, cs.Score, cs.ScoreMask) - adj = callResultRescoreAdj + adj := returnFeedsConstToIfAdj cs.Score, cs.ScoreMask = adjustScore(adj, cs.Score, cs.ScoreMask) } diff --git a/src/cmd/compile/internal/inline/inlheur/scoreadjusttyp_string.go b/src/cmd/compile/internal/inline/inlheur/scoreadjusttyp_string.go index 994a600f79..f5b8bf6903 100644 --- a/src/cmd/compile/internal/inline/inlheur/scoreadjusttyp_string.go +++ b/src/cmd/compile/internal/inline/inlheur/scoreadjusttyp_string.go @@ -20,29 +20,33 @@ func _() { _ = x[passFuncToNestedIndCallAdj-256] _ = x[passInlinableFuncToIndCallAdj-512] _ = x[passInlinableFuncToNestedIndCallAdj-1024] - _ = x[callResultRescoreAdj-2048] - _ = x[lastAdj-2048] + _ = x[returnFeedsConstToIfAdj-2048] + _ = x[returnFeedsFuncToIndCallAdj-4096] + _ = x[returnFeedsInlinableFuncToIndCallAdj-8192] + _ = x[returnFeedsConcreteToInterfaceCallAdj-16384] } var _scoreAdjustTyp_value = [...]uint64{ - 0x1, /* panicPathAdj */ - 0x2, /* initFuncAdj */ - 0x4, /* inLoopAdj */ - 0x8, /* passConstToIfAdj */ - 0x10, /* passConstToNestedIfAdj */ - 0x20, /* passConcreteToItfCallAdj */ - 0x40, /* passConcreteToNestedItfCallAdj */ - 0x80, /* passFuncToIndCallAdj */ - 0x100, /* passFuncToNestedIndCallAdj */ - 0x200, /* passInlinableFuncToIndCallAdj */ - 0x400, /* passInlinableFuncToNestedIndCallAdj */ - 0x800, /* callResultRescoreAdj */ - 0x800, /* lastAdj */ + 0x1, /* panicPathAdj */ + 0x2, /* initFuncAdj */ + 0x4, /* inLoopAdj */ + 0x8, /* passConstToIfAdj */ + 0x10, /* passConstToNestedIfAdj */ + 0x20, /* passConcreteToItfCallAdj */ + 0x40, /* passConcreteToNestedItfCallAdj */ + 0x80, /* passFuncToIndCallAdj */ + 0x100, /* passFuncToNestedIndCallAdj */ + 0x200, /* passInlinableFuncToIndCallAdj */ + 0x400, /* passInlinableFuncToNestedIndCallAdj */ + 0x800, /* returnFeedsConstToIfAdj */ + 0x1000, /* returnFeedsFuncToIndCallAdj */ + 0x2000, /* returnFeedsInlinableFuncToIndCallAdj */ + 0x4000, /* returnFeedsConcreteToInterfaceCallAdj */ } -const _scoreAdjustTyp_name = "panicPathAdjinitFuncAdjinLoopAdjpassConstToIfAdjpassConstToNestedIfAdjpassConcreteToItfCallAdjpassConcreteToNestedItfCallAdjpassFuncToIndCallAdjpassFuncToNestedIndCallAdjpassInlinableFuncToIndCallAdjpassInlinableFuncToNestedIndCallAdjcallResultRescoreAdjlastAdj" +const _scoreAdjustTyp_name = "panicPathAdjinitFuncAdjinLoopAdjpassConstToIfAdjpassConstToNestedIfAdjpassConcreteToItfCallAdjpassConcreteToNestedItfCallAdjpassFuncToIndCallAdjpassFuncToNestedIndCallAdjpassInlinableFuncToIndCallAdjpassInlinableFuncToNestedIndCallAdjreturnFeedsConstToIfAdjreturnFeedsFuncToIndCallAdjreturnFeedsInlinableFuncToIndCallAdjreturnFeedsConcreteToInterfaceCallAdj" -var _scoreAdjustTyp_index = [...]uint16{0, 12, 23, 32, 48, 70, 94, 124, 144, 170, 199, 234, 254, 261} +var _scoreAdjustTyp_index = [...]uint16{0, 12, 23, 32, 48, 70, 94, 124, 144, 170, 199, 234, 257, 284, 320, 357} func (i scoreAdjustTyp) String() string { var b bytes.Buffer diff --git a/src/cmd/compile/internal/inline/inlheur/scoring.go b/src/cmd/compile/internal/inline/inlheur/scoring.go index 5d026cb74c..fe2841797a 100644 --- a/src/cmd/compile/internal/inline/inlheur/scoring.go +++ b/src/cmd/compile/internal/inline/inlheur/scoring.go @@ -19,10 +19,35 @@ import ( // in which we'll adjust the score of a given callsite. type scoreAdjustTyp uint +// These constants capture the various ways in which the inliner's +// scoring phase can adjust a callsite score based on heuristics. They +// fall broadly into three categories: +// +// 1) adjustments based solely on the callsite context (ex: call +// appears on panic path) +// +// 2) adjustments that take into account specific interesting values +// passed at a call site (ex: passing a constant that could result in +// cprop/deadcode in the caller) +// +// 3) adjustments that take into account values returned from the call +// at a callsite (ex: call always returns the same inlinable function, +// and return value flows unmodified into an indirect call) +// +// For categories 2 and 3 above, each adjustment can have either a +// "must" version and a "may" version (but not both). Here the idea is +// that in the "must" version the value flow is unconditional: if the +// callsite executes, then the condition we're interested in (ex: +// param feeding call) is guaranteed to happen. For the "may" version, +// there may be control flow that could cause the benefit to be +// bypassed. const ( + // Catgegory 1 adjustments (see above) panicPathAdj scoreAdjustTyp = (1 << iota) initFuncAdj inLoopAdj + + // Category 2 adjustments (see above). passConstToIfAdj passConstToNestedIfAdj passConcreteToItfCallAdj @@ -31,8 +56,12 @@ const ( passFuncToNestedIndCallAdj passInlinableFuncToIndCallAdj passInlinableFuncToNestedIndCallAdj - callResultRescoreAdj - lastAdj scoreAdjustTyp = callResultRescoreAdj + + // Category 3 adjustments. + returnFeedsConstToIfAdj + returnFeedsFuncToIndCallAdj + returnFeedsInlinableFuncToIndCallAdj + returnFeedsConcreteToInterfaceCallAdj ) // This table records the specific values we use to adjust call @@ -42,18 +71,21 @@ const ( // what value for each one produces the best performance. var adjValues = map[scoreAdjustTyp]int{ - panicPathAdj: 40, - initFuncAdj: 20, - inLoopAdj: -5, - passConstToIfAdj: -20, - passConstToNestedIfAdj: -15, - passConcreteToItfCallAdj: -30, - passConcreteToNestedItfCallAdj: -25, - passFuncToIndCallAdj: -25, - passFuncToNestedIndCallAdj: -20, - passInlinableFuncToIndCallAdj: -45, - passInlinableFuncToNestedIndCallAdj: -40, - callResultRescoreAdj: 0, + panicPathAdj: 40, + initFuncAdj: 20, + inLoopAdj: -5, + passConstToIfAdj: -20, + passConstToNestedIfAdj: -15, + passConcreteToItfCallAdj: -30, + passConcreteToNestedItfCallAdj: -25, + passFuncToIndCallAdj: -25, + passFuncToNestedIndCallAdj: -20, + passInlinableFuncToIndCallAdj: -45, + passInlinableFuncToNestedIndCallAdj: -40, + returnFeedsConstToIfAdj: -15, + returnFeedsFuncToIndCallAdj: -25, + returnFeedsInlinableFuncToIndCallAdj: -40, + returnFeedsConcreteToInterfaceCallAdj: -25, } func adjValue(x scoreAdjustTyp) int { diff --git a/src/cmd/compile/internal/inline/inlheur/testdata/props/returns2.go b/src/cmd/compile/internal/inline/inlheur/testdata/props/returns2.go index 64f4628078..1bd23e74fa 100644 --- a/src/cmd/compile/internal/inline/inlheur/testdata/props/returns2.go +++ b/src/cmd/compile/internal/inline/inlheur/testdata/props/returns2.go @@ -12,7 +12,7 @@ package returns2 // returns2.go T_return_feeds_iface_call 18 0 1 // // {"Flags":0,"ParamFlags":[],"ResultFlags":[]} -// callsite: returns2.go:19:13|0 flagstr "" flagval 0 score -4 mask 2080 maskstr "passConcreteToItfCallAdj|callResultRescoreAdj" +// callsite: returns2.go:19:13|0 flagstr "" flagval 0 score 1 mask 16384 maskstr "returnFeedsConcreteToInterfaceCallAdj" // // func T_return_feeds_iface_call() { @@ -23,7 +23,7 @@ func T_return_feeds_iface_call() { // returns2.go T_multi_return_feeds_iface_call 29 0 1 // // {"Flags":0,"ParamFlags":[],"ResultFlags":[]} -// callsite: returns2.go:30:20|0 flagstr "" flagval 0 score -2 mask 2080 maskstr "passConcreteToItfCallAdj|callResultRescoreAdj" +// callsite: returns2.go:30:20|0 flagstr "" flagval 0 score 3 mask 16384 maskstr "returnFeedsConcreteToInterfaceCallAdj" // // func T_multi_return_feeds_iface_call() { @@ -34,12 +34,12 @@ func T_multi_return_feeds_iface_call() { // returns2.go T_returned_inlinable_func_feeds_indirect_call 41 0 1 // // {"Flags":0,"ParamFlags":[0],"ResultFlags":[]} -// callsite: returns2.go:42:18|0 flagstr "" flagval 0 score -43 mask 2560 maskstr "passInlinableFuncToIndCallAdj|callResultRescoreAdj" -// callsite: returns2.go:44:20|1 flagstr "" flagval 0 score -28 mask 2560 maskstr "passInlinableFuncToIndCallAdj|callResultRescoreAdj" +// callsite: returns2.go:42:18|0 flagstr "" flagval 0 score -51 mask 8200 maskstr "passConstToIfAdj|returnFeedsInlinableFuncToIndCallAdj" +// callsite: returns2.go:44:20|1 flagstr "" flagval 0 score -23 mask 8192 maskstr "returnFeedsInlinableFuncToIndCallAdj" // // func T_returned_inlinable_func_feeds_indirect_call(q int) { - f := returnsFunc() + f := returnsFunc(10) f(q) f2 := returnsFunc2() f2(q) @@ -48,7 +48,7 @@ func T_returned_inlinable_func_feeds_indirect_call(q int) { // returns2.go T_returned_noninlineable_func_feeds_indirect_call 54 0 1 // // {"Flags":0,"ParamFlags":[0],"ResultFlags":[]} -// callsite: returns2.go:55:30|0 flagstr "" flagval 0 score -23 mask 2176 maskstr "passFuncToIndCallAdj|callResultRescoreAdj" +// callsite: returns2.go:55:30|0 flagstr "" flagval 0 score -23 mask 4096 maskstr "returnFeedsFuncToIndCallAdj" // // func T_returned_noninlineable_func_feeds_indirect_call(q int) { @@ -59,7 +59,7 @@ func T_returned_noninlineable_func_feeds_indirect_call(q int) { // returns2.go T_multi_return_feeds_indirect_call 65 0 1 // // {"Flags":0,"ParamFlags":[0],"ResultFlags":[]} -// callsite: returns2.go:66:29|0 flagstr "" flagval 0 score -26 mask 2560 maskstr "passInlinableFuncToIndCallAdj|callResultRescoreAdj" +// callsite: returns2.go:66:29|0 flagstr "" flagval 0 score -21 mask 8192 maskstr "returnFeedsInlinableFuncToIndCallAdj" // // func T_multi_return_feeds_indirect_call(q int) { @@ -70,7 +70,7 @@ func T_multi_return_feeds_indirect_call(q int) { // returns2.go T_return_feeds_ifswitch 76 0 1 // // {"Flags":0,"ParamFlags":[0],"ResultFlags":[0]} -// callsite: returns2.go:77:14|0 flagstr "" flagval 0 score 5 mask 2056 maskstr "passConstToIfAdj|callResultRescoreAdj" +// callsite: returns2.go:77:14|0 flagstr "" flagval 0 score 10 mask 2048 maskstr "returnFeedsConstToIfAdj" // // func T_return_feeds_ifswitch(q int) int { @@ -87,7 +87,7 @@ func T_return_feeds_ifswitch(q int) int { // returns2.go T_multi_return_feeds_ifswitch 93 0 1 // // {"Flags":0,"ParamFlags":[0],"ResultFlags":[0]} -// callsite: returns2.go:94:21|0 flagstr "" flagval 0 score 4 mask 2056 maskstr "passConstToIfAdj|callResultRescoreAdj" +// callsite: returns2.go:94:21|0 flagstr "" flagval 0 score 9 mask 2048 maskstr "returnFeedsConstToIfAdj" // // func T_multi_return_feeds_ifswitch(q int) int { @@ -126,19 +126,19 @@ func T_two_calls_feed_ifswitch(q int) int { // returns2.go T_chained_indirect_call 132 0 1 // // {"Flags":0,"ParamFlags":[0,0],"ResultFlags":[]} -// callsite: returns2.go:135:18|0 flagstr "" flagval 0 score -43 mask 2560 maskstr "passInlinableFuncToIndCallAdj|callResultRescoreAdj" +// callsite: returns2.go:135:18|0 flagstr "" flagval 0 score -31 mask 8192 maskstr "returnFeedsInlinableFuncToIndCallAdj" // // func T_chained_indirect_call(x, y int) { // Here 'returnsFunc' returns an inlinable func that feeds // directly into a call (no named intermediate). - G += returnsFunc()(x + y) + G += returnsFunc(x - y)(x + y) } // returns2.go T_chained_conc_iface_call 144 0 1 // // {"Flags":0,"ParamFlags":[0,0],"ResultFlags":[]} -// callsite: returns2.go:148:8|0 flagstr "" flagval 0 score -4 mask 2080 maskstr "passConcreteToItfCallAdj|callResultRescoreAdj" +// callsite: returns2.go:148:8|0 flagstr "" flagval 0 score 1 mask 16384 maskstr "returnFeedsConcreteToInterfaceCallAdj" // // func T_chained_conc_iface_call(x, y int) { @@ -148,7 +148,10 @@ func T_chained_conc_iface_call(x, y int) { newBar(10).Plark().Plark() } -func returnsFunc() func(int) int { +func returnsFunc(x int) func(int) int { + if x < 0 { + G++ + } return adder } -- 2.48.1