Skip to content

Conversation

@github-actions
Copy link
Contributor

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

Backport of #106648 to release/9.0

/cc @amanasifkhalid @SwapnilGaikwad

Customer Impact

  • Customer reported
  • Found internally

The ARM SVE instruction set has a special SIMD register called the first-faulting register (FFR), where if a vectorized load operation causes a page fault, the lanes in the FFR corresponding to the offending loads are zeroed. For SVE intrinsics with first-faulting semantics, if a load faults, the remaining load operations are skipped, and the corresponding FFR lanes are zeroed. For SVE intrinsics with non-faulting semantics, execution continues in spite of any faults, but the FFR is updated accordingly.

Because these intrinsics implicitly modify the FFR, the JIT has to be judicious in modelling the state of this register. When we initially implemented the first-faulting APIs shipping in .NET 9, we added special cases in lowering and LSRA to track the FFR effects of these intrinsics. However, we did not initially replicate this handling when we added the non-faulting variants of these APIs. This PR fixes that, and also updates our existing non-faulting test suite to verify the FFR is being tracked correctly.

Regression

  • Yes
  • No

This PR fixes scenarios specific to SVE, which is newly supported in .NET 9.

Testing

Our existing Sve.LoadVectorNonFaulting* test suite has been updated to check the FFR's state for correctness.

Risk

Low. This fix pertains to SVE support, which is experimental in .NET 9.

@ghost ghost added the area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI label Aug 21, 2024
@dotnet-policy-service
Copy link
Contributor

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

@amanasifkhalid
Copy link
Contributor

@a74nh PTAL, thanks!

Copy link
Contributor

@a74nh a74nh left a comment

Choose a reason for hiding this comment

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

Agree with this. The missing FFR tracking means the LoadVector*NonFaulting APIs are not functionally correct.
It will be rare that someone would use FFR and LoadVector*NonFaulting together, however if FFR does get out of sync it will cause bigger issues. So, this does need fixing ASAP.

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.

approved. we can merge when ready

@jeffschwMSFT jeffschwMSFT added the Servicing-approved Approved for servicing release label Aug 22, 2024
@jeffschwMSFT jeffschwMSFT merged commit 7384dc4 into release/9.0 Aug 22, 2024
@amanasifkhalid amanasifkhalid deleted the backport/pr-106648-to-release/9.0 branch August 22, 2024 19:14
@github-actions github-actions bot locked and limited conversation to collaborators Sep 22, 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