-
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: functions with type parameters cannot inline multiple levels deep across packages #56280
Comments
cc @golang/compiler |
This feels like an export data issue to me. I see:
But only the second actually happens across packages. Is the inline body of |
Instantiated bodies are not included in unified IR's export data format. It only exports generic bodies, and then reinstantiates them as appropriate. Similarly, it also reanalyzes for inlining and escape analysis. The problem here is an imported, non-generic, inlinable function F calls a generic, inlinable function G[uintptr]. However, this isn't discovered until we get to inlining, lazily read in F's function body, and re-discover the call to G[uintptr] and instantiate it (as G[go.shape.uintptr], along with an appropriate runtime dictionary). At this point, we've already missed the opportunity to call CanInline on G[go.shape.uintptr] (though probably even if we created it earlier, ir.VisitFuncsBottomUp wouldn't realize it's transitively relevant to inlining F). Probably we need to manually call CanInline on instantiated, imported generic functions. |
Change https://go.dev/cl/443535 mentions this issue: |
Will this bugfix be backported to go 1.19? |
No, normally a performance fix does not qualify for a backport. |
Also add the missing HugePage equivalents of Page functions. The fix for golang/go#56280, which first appears in Go 1.20, is needed to prevent this CL from regressing performance: Before fix: ``` TEXT gvisor/pkg/sentry/mm/mm.(*MemoryManager).SetNumaPolicy(SB) gvisor/pkg/sentry/mm/syscalls.go ... addr.go:75 0x7f000054e35e 488d053bc2b100 LEAQ gvisor/pkg/hostarch/hostarch..dict.IsPageAligned[gvisor/pkg/hostarch/hostarch.Addr](SB), AX addr.go:75 0x7f000054e365 e8961cc4ff CALL gvisor/pkg/hostarch/hostarch.IsPageAligned[go.shape.uintptr](SB) ``` After fix: ``` TEXT gvisor/pkg/sentry/mm/mm.(*MemoryManager).SetNumaPolicy(SB) gvisor/pkg/sentry/mm/syscalls.go ... addr.go:75 0x7f00004da61a 90 NOPL addr.go:75 0x7f00004da61b 0f1f440000 NOPL 0(AX)(AX*1) sizes_util.go:57 0x7f00004da620 48f7c3ff0f0000 TESTQ $0xfff, BX syscalls.go:1021 0x7f00004da627 0f855f010000 JNE 0x7f00004da78c ``` PiperOrigin-RevId: 507608080
What version of Go are you using (
go version
)?Does this issue reproduce with the latest release?
Yes (1.19)
What operating system and processor architecture are you using (
go env
)?go env
OutputWhat did you do?
Build https://go.dev/play/p/X7fahXzzE38
What did you expect to see?
result = input.ViaGenericPageOffset
inlined as a single AND.What did you see instead?
input.ViaGenericPageOffset()
is inlined aspkg.GenericPageOffset[example.com/pkg.Addr](input)
, even though the non-inline version ofpkg.Addr.ViaGenericPageOffset
fully inlinesGenericPageOffset
.If the contents of
pkg
are moved into packagemain
(https://go.dev/play/p/8WY0mElNpV3), theninput.ViaGenericPageOffset
is inlined intomain.main
exactly the same asinput.ViaConcretePageOffset
. I do not expect this behavior to depend on which package the code is in.The text was updated successfully, but these errors were encountered: