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] Ensure that constant folding of bitwise operations for float/double are bitwise #106830

Merged
merged 5 commits into from
Aug 23, 2024

Conversation

github-actions[bot]
Copy link
Contributor

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

Backport of #106711 to release/9.0

/cc @tannergooding

Customer Impact

  • Customer reported
  • Found internally

Found via Fuzzlyn: #106610

Customers doing bitwise operations on floating-point vectors and constants may see unexpected results if the input was a signaling NaN.

Regression

  • Yes
  • No

This is a difference in behavior from prior releases and while technically within the behavior allowed by spec, it is an unexpected behavior change and doesn't follow the recommended practice.

The behavior change was introduced because the constant folding of SIMD data is handled by decomposition over the scalar elements. For floating-point, any floating-point instruction which encounters a scalar signaling NaN will cause the signal to be "processed" (which is a no-op in .NET as we disallow IEEE 754 floating-point exceptions) and the NaN will then be converted into an equivalent quiet NaN. This differs in behavior from the native SIMD behavior which will not convert the signaling NaN into a quiet NaN.

The fix is then to instead recognize such SIMD bitwise operations and ensure they get processed as integer scalars instead, never producing an intermediate floating-point scalar, so that this NaN handling cannot occur.

Testing

A regression test was added for the scenario produced by Fuzzlyn. Additional manual validation was done for each of the bitwise operations on x86, x64, and Arm64 for the relevant ISA levels to ensure that the expected results were produced for signaling NaN inputs.

Risk

Low.

This is a new regression related to an edge case with constant folding. Signaling NaN are not typical in the first place and the overall handling remains the same as before, just without producing an intermediate floating-point scalar.

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

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

@tannergooding
Copy link
Member

CC. @dotnet/jit-contrib, @AndyAyersMS for sign-off

This backports the fix for #106711 to .NET 9

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. once we have a code review and CI is good, we can merge

@jeffschwMSFT jeffschwMSFT added the Servicing-approved Approved for servicing release label Aug 22, 2024
@jeffschwMSFT
Copy link
Member

@tannergooding can you take a look at the failures in CI?

@tannergooding
Copy link
Member

Looks like #100558 and a nativeruntimeeventsource timeout.

The latter has an older tracking issue for Mono already (#92733), but not one for RyuJIT. There's also at least one other failure that was showing up for this test on Mono: #90605 and a few issues discussing possible rare race conditions.

I've requeued the jobs and will get an issue logged for the nativeruntimeeventsource timeout

@tannergooding
Copy link
Member

@jeffschwMSFT, should be ready to merge. Test passed on rerun, but I made sure tracking issues were logged for the failures (links are automatically given by GitHub just above).

@jeffschwMSFT jeffschwMSFT merged commit 0ee150e into release/9.0 Aug 23, 2024
9 checks passed
@tannergooding tannergooding deleted the backport/pr-106711-to-release/9.0 branch August 23, 2024 19:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
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.

3 participants