-
Notifications
You must be signed in to change notification settings - Fork 17.9k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
cmd/compile: PGO fails to do multiple levels of inlining into a single function #69046
Comments
Getting the original name looks somewhat painful, as it looks like the only reference we keep to it is a reference to the inline tree via an inline mark statement in the expression init: https://cs.opensource.google/go/go/+/master:src/cmd/compile/internal/noder/reader.go;drc=4e1cc09f8b9bcef2b6d0839a7d0026b50c21998b;bpv=1;bpt=1;l=3513 |
Consider the following inlining chain: F1() -> F2() -> F3(). F3 is a hot function that can be sequentially inlined into F1 after F2. During the inline cost checking, it should be determined whether the edge from F2 to F3 is hot, but instead the edge from F1 to F3 is checked, and this leads to incorrect inlining decisions. This CL fixes the problem of searching in PGO edge map. It considers true parent caller using InlTree. Fixes golang#69046
Change https://go.dev/cl/626295 mentions this issue: |
Reproducer:
https://go.dev/play/p/QxBt13wV3Uh
Run the benchmark to collect a profile, and then use that as a PGO profile:
The
hot-budget
lines note that PGO allows inliningb
intoa
anda
intoFoo
.dep/dep.go:32:3: inlining call to a
indicates thata
was inlined intoFoo
, but there should then be a subsequent inlineb
intoFoo
. The output makes this a bit confusing, but objdump makes it clear thatb
was not inlined:A bit of custom logging in the compiler makes it more clear what is happening:
It looks like what is happening here is:
a
in inlined intoFoo
.b
intoFoo
(from the inlined body ofa
).canInlineCallExpr
checksinlineCostOK
ofFoo -> b
. This consults the PGO edge map, which does not contain a hot edge fromFoo -> b
because PGO includes inline calls in edges. i.e., it has hot edgesFoo -> a
anda -> b
.I believe what should happen is if inlining a call inside of an
ir.InlinedCallExpr
, we should use the original name of theInlinedCallExpr
to consult the PGO edge map.cc @cherrymui
The text was updated successfully, but these errors were encountered: