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/9.0] Arm64: Use TYP_SIMD8 for AddAcross #108805

Merged
merged 6 commits into from
Oct 14, 2024

Conversation

github-actions[bot]
Copy link
Contributor

@github-actions github-actions bot commented Oct 11, 2024

Backport of #108727 to release/9.0

/cc @kunalspathak

Customer Impact

  • Customer reported
  • Found internally

When creating AddAcross() we were wrongly setting the wrong return type to TYP_SIMD16 instead of TYP_SIMD8. When customer use AdvSimd.Arm64.AddAcross() directly or indirectly when using Vector128.Sum() (which we translate into AddAcross() as part of #97460). The effect might be visible when during code generation, we would wrongly produce a value in v128 register instead of v64 and take dependence on data present in upper-half of v128 because of that, which can be garbage, leading to silent codegen issues.

Regression

  • Yes
  • No

[If yes, specify when the regression was introduced. Provide the PR or commit if known.]

Testing

Verified locally and added test cases

Risk

Low: This affects AddAcross() intrinsic Arm64.

IMPORTANT: If this backport is for a servicing release, please verify that:

  • The PR target branch is release/X.0-staging, not release/X.0.

  • If the change touches code that ships in a NuGet package, you have added the necessary package authoring and gotten it explicitly reviewed.

@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 Oct 11, 2024
Copy link
Contributor

Tagging subscribers to this area: @JulieLeeMSFT, @jakobbotsch
See info in area-owners.md if you want to be subscribed.

@kunalspathak
Copy link
Member

@dotnet/jit-contrib @tannergooding

Copy link
Member

@jeffschwMSFT jeffschwMSFT left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm. please get a code review. we will take for consideration in 9 GA

@kunalspathak
Copy link
Member

The failure seems to be known failure

image

not sure why Build Analysis is still red.

@jeffschwMSFT jeffschwMSFT added Servicing-approved Approved for servicing release and removed Servicing-consider Issue for next servicing release review labels Oct 11, 2024
@jeffschwMSFT jeffschwMSFT added this to the 9.0.0 milestone Oct 11, 2024
@jeffschwMSFT
Copy link
Member

please get a code review and then we can merge

@carlossanlop carlossanlop merged commit c2da91e into release/9.0 Oct 14, 2024
98 of 99 checks passed
@carlossanlop carlossanlop deleted the backport/pr-108727-to-release/9.0 branch October 14, 2024 16:27
@github-actions github-actions bot locked and limited conversation to collaborators Nov 14, 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.

4 participants