Skip to content
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.

Sign up for GitHub

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

Jump to bottom

cmd/compile: Go 1.22 build fails with 1.21 PGO profile on internal/saferio change [1.22 backport] #65618

Closed
gopherbot opened this issue Feb 8, 2024 · 2 comments
Closed

cmd/compile: Go 1.22 build fails with 1.21 PGO profile on internal/saferio change [1.22 backport] #65618

gopherbot opened this issue Feb 8, 2024 · 2 comments
Labels
CherryPickApproved Used during the release process for point releases compiler/runtime Issues related to the Go compiler and/or runtime.
Milestone
Go1.22.1

Comments

@gopherbot
Copy link
Contributor

gopherbot commented Feb 8, 2024 •
edited by dmitshur
Loading

@prattmic requested issue #65615 to be considered for backport to the next 1.22 minor release.

@gopherbot Please backport to 1.21 and 1.22. This is a build failure with PGO if a hot function is changed from non-generic to generic. Only workarounds are to rename the symbol or disable PGO.
@gopherbot gopherbot added the CherryPickCandidate Used during the release process for point releases label Feb 8, 2024
@gopherbot gopherbot mentioned this issue Feb 8, 2024
Closed
@gopherbot gopherbot added the compiler/runtime Issues related to the Go compiler and/or runtime. label Feb 8, 2024
@gopherbot gopherbot added this to the Go1.22.1 milestone Feb 8, 2024
@gopherbot
Copy link
Contributor Author

Change https://go.dev/cl/563016 mentions this issue: [release-branch.go1.22] cmd/compile: fail noder.LookupFunc gracefully if function generic

@thanm thanm added CherryPickApproved Used during the release process for point releases and removed CherryPickCandidate Used during the release process for point releases labels Feb 14, 2024
@gopherbot
Copy link
Contributor Author

Closed by merging 6cbe522 to release-branch.go1.22.

gopherbot pushed a commit that referenced this issue Feb 16, 2024
@prattmic @thanm
[release-branch.go1.22] cmd/compile: fail noder.LookupFunc gracefully…
6cbe522 
… if function generic

PGO uses noder.LookupFunc to look for devirtualization targets in
export data.  LookupFunc does not support type-parameterized
functions, and will currently fail the build when attempting to lookup
a type-parameterized function because objIdx is passed the wrong
number of type arguments.

This doesn't usually come up, as a PGO profile will report a generic
function with a symbol name like Func[.go.shape.foo]. In export data,
this is just Func, so when we do LookupFunc("Func[.go.shape.foo]")
lookup simply fails because the name doesn't exist.

However, if Func is not generic when the profile is collected, but the
source has since changed to make Func generic, then LookupFunc("Func")
will find the object successfully, only to fail the build because we
failed to provide type arguments.

Handle this with a objIdxMayFail, which allows graceful failure if the
object requires type arguments.

Bumping the language version to 1.21 in pgo_devirtualize_test.go is
required for type inference of the uses of mult.MultFn in
cmd/compile/internal/test/testdata/pgo/devirtualize/devirt_test.go.

For #65615.
Fixes #65618.

Change-Id: I84d9344840b851182f5321b8f7a29a591221b29f
Reviewed-on: https://go-review.googlesource.com/c/go/+/562737
Reviewed-by: Cherry Mui <cherryyz@google.com>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
(cherry picked from commit 532c6f1)
Reviewed-on: https://go-review.googlesource.com/c/go/+/563016
bradfitz pushed a commit to tailscale/go that referenced this issue Mar 5, 2024
@prattmic @bradfitz
[release-branch.go1.22] cmd/compile: fail noder.LookupFunc gracefully…
05bdd83 
… if function generic

PGO uses noder.LookupFunc to look for devirtualization targets in
export data.  LookupFunc does not support type-parameterized
functions, and will currently fail the build when attempting to lookup
a type-parameterized function because objIdx is passed the wrong
number of type arguments.

This doesn't usually come up, as a PGO profile will report a generic
function with a symbol name like Func[.go.shape.foo]. In export data,
this is just Func, so when we do LookupFunc("Func[.go.shape.foo]")
lookup simply fails because the name doesn't exist.

However, if Func is not generic when the profile is collected, but the
source has since changed to make Func generic, then LookupFunc("Func")
will find the object successfully, only to fail the build because we
failed to provide type arguments.

Handle this with a objIdxMayFail, which allows graceful failure if the
object requires type arguments.

Bumping the language version to 1.21 in pgo_devirtualize_test.go is
required for type inference of the uses of mult.MultFn in
cmd/compile/internal/test/testdata/pgo/devirtualize/devirt_test.go.

For golang#65615.
Fixes golang#65618.

Change-Id: I84d9344840b851182f5321b8f7a29a591221b29f
Reviewed-on: https://go-review.googlesource.com/c/go/+/562737
Reviewed-by: Cherry Mui <cherryyz@google.com>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
(cherry picked from commit 532c6f1)
Reviewed-on: https://go-review.googlesource.com/c/go/+/563016
romaindoumenc pushed a commit to TroutSoftware/go that referenced this issue Mar 6, 2024
@prattmic @romaindoumenc
[release-branch.go1.22] cmd/compile: fail noder.LookupFunc gracefully…
a781b9c 
… if function generic

PGO uses noder.LookupFunc to look for devirtualization targets in
export data.  LookupFunc does not support type-parameterized
functions, and will currently fail the build when attempting to lookup
a type-parameterized function because objIdx is passed the wrong
number of type arguments.

This doesn't usually come up, as a PGO profile will report a generic
function with a symbol name like Func[.go.shape.foo]. In export data,
this is just Func, so when we do LookupFunc("Func[.go.shape.foo]")
lookup simply fails because the name doesn't exist.

However, if Func is not generic when the profile is collected, but the
source has since changed to make Func generic, then LookupFunc("Func")
will find the object successfully, only to fail the build because we
failed to provide type arguments.

Handle this with a objIdxMayFail, which allows graceful failure if the
object requires type arguments.

Bumping the language version to 1.21 in pgo_devirtualize_test.go is
required for type inference of the uses of mult.MultFn in
cmd/compile/internal/test/testdata/pgo/devirtualize/devirt_test.go.

For golang#65615.
Fixes golang#65618.

Change-Id: I84d9344840b851182f5321b8f7a29a591221b29f
Reviewed-on: https://go-review.googlesource.com/c/go/+/562737
Reviewed-by: Cherry Mui <cherryyz@google.com>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
(cherry picked from commit 532c6f1)
Reviewed-on: https://go-review.googlesource.com/c/go/+/563016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
CherryPickApproved Used during the release process for point releases compiler/runtime Issues related to the Go compiler and/or runtime.
Projects
None yet
Milestone
Go1.22.1
Development

No branches or pull requests
2 participants
@gopherbot @thanm