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

[release/8.0-staging] Ensure that AdvSimd.Insert doesn't zero out the upper bits #107091

Merged

Conversation

github-actions[bot]
Copy link
Contributor

@github-actions github-actions bot commented Aug 28, 2024

Backport of #106981 to release/8.0-staging

/cc @tannergooding

Customer Impact

  • Customer reported
  • Found internally

Found via Fuzzlyn: #106079

Developers who are using AdvSimd.Insert to insert a constant floating-point value may see incorrect codegen on Arm64.

Regression

  • Yes
  • No

This bug was introduced in .NET 5 when the API was originally added: #35030

Testing

An explicit regression test was added covering the scenario. Additional manual testing of the patterns that are known to be problematic was completed on an Arm64 device. Similar patterns were also validated on x64 and the code was audited to identify if similar issues could exist.

Risk

Low. Most user code is doing insertions via the cross platform API, vector.WithElement instead which uses a different intrinsic (InsertSelectedScalar) to do the insertion, which does not have this same problem.

@dotnet-issue-labeler dotnet-issue-labeler bot added the area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI label Aug 28, 2024
@tannergooding
Copy link
Member

CC. @jakobbotsch, @dotnet/jit-contrib for review

This is a backport of #106981 to .NET 8

@carlossanlop
Copy link
Member

This is missing Tactics approval.

Friendly reminder that Code Complete for the October Release is September 9. If we want this fix to be included in that release, please merge this PR before that date.

@JulieLeeMSFT JulieLeeMSFT added the Servicing-consider Issue for next servicing release review label Sep 9, 2024
@jeffschwMSFT jeffschwMSFT added Servicing-approved Approved for servicing release and removed Servicing-consider Issue for next servicing release review labels Sep 12, 2024
@jeffschwMSFT
Copy link
Member

@tannergooding please take a look at the PR failures

@tannergooding
Copy link
Member

Build was deleted, updated branch so it can rerun and investigation can happen.

@tannergooding
Copy link
Member

@jeffschwMSFT CI passed on rerun, should be good to merge.

@carlossanlop carlossanlop merged commit 62ef9b8 into release/8.0-staging Sep 19, 2024
121 checks passed
@carlossanlop carlossanlop deleted the backport/pr-106981-to-release/8.0-staging branch September 19, 2024 18:15
@carlossanlop
Copy link
Member

@jeffschwMSFT CI passed on rerun, should be good to merge.

@tannergooding FYI - All devs are allowed to merge PRs to the release/X.0-staging branches. Admin merges are only needed in the base branches (release/X.0).

@github-actions github-actions bot locked and limited conversation to collaborators Oct 20, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI Servicing-approved Approved for servicing release
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants