]> Cypherpunks.ru repositories - gostls13.git/commit
cmd/compile: synchronize inlinability logic between hairyVisitor and mkinlcall
authorMichael Pratt <mpratt@google.com>
Mon, 10 Apr 2023 21:11:39 +0000 (17:11 -0400)
committerMichael Pratt <mpratt@google.com>
Thu, 12 Oct 2023 18:12:19 +0000 (18:12 +0000)
commitb56645a87b28840a180d64077877cb46570b4176
tree39b10b0b6a4ffa3eac9c38b44582e3cc4486a8fa
parent657c885fb9278f03d5b18bfb7eeca98c25ef67ac
cmd/compile: synchronize inlinability logic between hairyVisitor and mkinlcall

When computing function cost, hairyVisitor.doNode has two primary cases
for determining the cost of a call inside the function:

* Normal calls are simply cost 57.
* Calls that can be inlined have the cost of the inlined function body,
  since that body will end up in this function.

Determining which "calls can be inlined" is where this breaks down.
doNode simply assumes that any function with `fn.Inl != nil` will get
inlined. However, this are more complex in mkinlcall, which has a
variety of cases where it may not inline.

For standard builds, most of these reasons are fairly rare (recursive
calls, calls to runtime functions in instrumented builds, etc), so this
simplification isn't too build.

However, for PGO builds, any function involved in at least one inlinable
hot callsite will have `fn.Inl != nil`, even though mkinlcall will only
inline at the hot callsites. As a result, cold functions calling hot
functions will use the (potentially very large) hot function inline body
cost in their call budget. This could make these functions too expensive
to inline even though they won't actually inline the hot function.

Handle this case plus the other inlinability cases (recursive calls,
etc) by consolidating mkinlcall's inlinability logic into
canInlineCallExpr, which is shared by doNode.

mkinlcall and doNode now have identical logic, except for one case: we
check for recursive cycles via inlined functions by looking at the
inline tree. Since we haven't actually done any inlining yet when in
doNode, we will miss those cases.

This CL doesn't change any inlining decisions in a standard build of the
compiler.

In the new inliner, the inlining decision is also based on the call
site, so this synchronization is also helpful.

Fixes #59484

Change-Id: I6ace66e37d50526535972215497ef75cd71f8b9d
Reviewed-on: https://go-review.googlesource.com/c/go/+/483196
Reviewed-by: Than McIntosh <thanm@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Cherry Mui <cherryyz@google.com>
src/cmd/compile/internal/inline/inl.go