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.

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: encoding/binary.PutUint16 sometimes doesn't write [1.19 backport] #59373

Closed
gopherbot opened this issue Apr 1, 2023 · 2 comments
Labels
CherryPickApproved Used during the release process for point releases compiler/runtime Issues related to the Go compiler and/or runtime.
Milestone

Comments

@gopherbot
Copy link

@randall77 requested issue #59367 to be considered for backport to the next 1.19 minor release.

@gopherbot please open backport issues for 1.20 and 1.19.

@gopherbot gopherbot added the CherryPickCandidate Used during the release process for point releases label Apr 1, 2023
@gopherbot gopherbot added the compiler/runtime Issues related to the Go compiler and/or runtime. label Apr 1, 2023
@gopherbot gopherbot added this to the Go1.19.8 milestone Apr 1, 2023
@mknyszek mknyszek modified the milestones: Go1.19.8, Go1.19.9 Apr 4, 2023
@gopherbot
Copy link
Author

Change https://go.dev/cl/483177 mentions this issue: [release-branch.go1.19] cmd/compile: use correct type for byteswaps on multi-byte stores

@dr2chase dr2chase added the CherryPickApproved Used during the release process for point releases label Apr 12, 2023
@gopherbot gopherbot removed the CherryPickCandidate Used during the release process for point releases label Apr 12, 2023
@gopherbot
Copy link
Author

Closed by merging ca9947d to release-branch.go1.19.

gopherbot pushed a commit that referenced this issue Apr 24, 2023
…n multi-byte stores

Use the type of the store for the byteswap, not the type of the
store's value argument.

Normally when we're storing a 16-bit value, the value being stored is
also typed as 16 bits. But sometimes it is typed as something smaller,
usually because it is the result of an upcast from a smaller value,
and that upcast needs no instructions.

If the type of the store's arg is thinner than the type being stored,
and the byteswap'd value uses that thinner type, and the byteswap'd
value needs to be spilled & restored, that spill/restore happens using
the thinner type, which causes us to lose some of the top bits of the
value.

Fixes #59373

Change-Id: If6ce1e8a76f18bf8e9d79871b6caa438bc3cce4d
Reviewed-on: https://go-review.googlesource.com/c/go/+/481395
Reviewed-by: David Chase <drchase@google.com>
Reviewed-by: Cherry Mui <cherryyz@google.com>
Run-TryBot: Keith Randall <khr@golang.org>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-on: https://go-review.googlesource.com/c/go/+/483177
Auto-Submit: Dmitri Shuralyov <dmitshur@google.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CherryPickApproved Used during the release process for point releases compiler/runtime Issues related to the Go compiler and/or runtime.
Projects
None yet
Development

No branches or pull requests

3 participants