Preserve side effects in SIMD classification helpers - #132921
Open
tannergooding wants to merge 1 commit into
Open
Preserve side effects in SIMD classification helpers#132921tannergooding wants to merge 1 commit into
tannergooding wants to merge 1 commit into
Conversation
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
github-actions
Bot
added
the
area-CodeGen-coreclr
|
Azure Pipelines: Successfully started running 5 pipeline(s). 11 pipeline(s) were filtered out due to trigger conditions. There may be pipelines that require an authorized user to comment /azp run to run. |
Contributor
|
Tagging subscribers to this area: @JulieLeeMSFT, @jakobbotsch |
Contributor
There was a problem hiding this comment.
Copilot review overview
🟢 Approval recommended
Review tier: Lite
Findings: None
What changed in this PR
This PR fixes a JIT correctness issue where certain SIMD classification intrinsics could fold to a constant result and inadvertently drop operand side effects (notably exceptions). The fix wraps the folded constant results with extracted side effects from the operand, and adds a targeted regression test to ensure the operand is still evaluated.
Changes:
- Update JIT SIMD classification helpers to preserve operand side effects when folding to constant results (e.g., integral-type cases).
- Add a new JIT regression test that validates exception-bearing operands are not elided across nine classification operations.
- Register the new regression test source file in the merged regression test project.
| File | Description |
|---|---|
| src/coreclr/jit/gentree.cpp | Wrap constant-folded SIMD classification results with extracted side effects from the operand to prevent eliding exceptions/calls. |
| src/tests/JIT/Regression/JitBlue/Runtime_132902/Runtime_132902.cs | Adds regression coverage using Vector128.WithElement with an out-of-range index to ensure operand exceptions are preserved across classification intrinsics. |
| src/tests/JIT/Regression/Regression_ro_2.csproj | Includes the new regression test in the merged JIT regression project build. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #132902
SIMD classification helpers that produce constant results now preserve side effects from their operands. This covers all nine affected classification operations rather than only the reported
IsInfinitycase.Adds regression coverage using an exception-bearing
Vector128.WithElementoperand, which exercises the shared x64 and WASM import shape.Note
This pull request was created with GitHub Copilot.