Skip to content

Conversation

kunalspathak
Copy link
Contributor

As pointed in #112264 (comment), the mask values returned by helper methods GetMaskDouble() and GetMaskSingle() are incorrect and were getting returned using BitConverter.DoubleToInt64Bits() and BitConverter.SingleToInt32Bits() respectively. This might make a lane active or inactive, depending on the value of bit 0 (on which mask registers operate on). However, in the validation, we just assume that anything that is non-zero is active. Fix it by returning strictly either 0 or 1.

Fixes: #112264

These might also be related certain functional incorrect failures seen in #112377. Note that in the issue, there was also a reference of getting Index was outside the bounds of the array which might be unrelated and is not addressed by this fix.

@Copilot Copilot AI review requested due to automatic review settings April 4, 2025 05:39
@ghost ghost added the area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI label Apr 4, 2025
Copy link
Contributor

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

Copy link
Contributor

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Copilot reviewed 1 out of 1 changed files in this pull request and generated 1 comment.

Comments suppressed due to low confidence (2)

src/tests/JIT/HardwareIntrinsics/Arm/Shared/Helpers.cs:7747

  • Directly returning the modulo result fixes the incorrect mask value, but confirm that TestLibrary.Generator.GetInt32() always produces non-negative values to ensure the returned mask is strictly 0 or 1.
return (float)(TestLibrary.Generator.GetInt32()%(int)2);

src/tests/JIT/HardwareIntrinsics/Arm/Shared/Helpers.cs:7745

  • Consider adding unit tests to validate that both getMaskSingle() and getMaskDouble() always return 0 or 1 as intended.
public static float getMaskSingle() { ... }

@kunalspathak
Copy link
Contributor Author

@dotnet/jit-contrib @dotnet/arm64-contrib

Copy link
Contributor

@amanasifkhalid amanasifkhalid left a comment

Choose a reason for hiding this comment

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

Looks like there are some new CI failures to sort through

@kunalspathak kunalspathak merged commit 7422c52 into dotnet:main Apr 4, 2025
74 of 76 checks passed
@kunalspathak kunalspathak deleted the sve-testcases branch April 4, 2025 20:39
@github-actions github-actions bot locked and limited conversation to collaborators May 5, 2025
@jozkee
Copy link
Member

jozkee commented Jun 9, 2025

/backport to release/9.0-staging

Copy link
Contributor

github-actions bot commented Jun 9, 2025

Started backporting to release/9.0-staging: https://github.com/dotnet/runtime/actions/runs/15539869299

@github-actions github-actions bot unlocked this conversation Jun 9, 2025
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jun 9, 2025
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
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Test failure: _Sve_ro::JIT.HardwareIntrinsics.Arm._Sve.Program.Sve_GatherVector_Bases_double_ulong()
3 participants