ARM64-SVE: Add `TrigonometricSelectCoefficient`, `TrigonometricStartingValue`#104681

amanasifkhalid · 2024-07-10T15:20:37Z

Part of #99957. @dotnet/arm64-contrib PTAL, thanks!

Test output:

Starting test: .\Core_Root\corerun.exe .\HardwareIntrinsics_Arm_ro\HardwareIntrinsics_Arm_ro.dll Sve_Trigonometric
===================Running default===================
------------------- {} -------------------
Passed test: _Sve_ro::JIT.HardwareIntrinsics.Arm._Sve.Program.Sve_TrigonometricSelectCoefficient_float_uint() : 24
Passed test: _Sve_ro::JIT.HardwareIntrinsics.Arm._Sve.Program.Sve_TrigonometricSelectCoefficient_double_ulong() : 24
Passed test: _Sve_ro::JIT.HardwareIntrinsics.Arm._Sve.Program.Sve_TrigonometricStartingValue_float_uint() : 24
Passed test: _Sve_ro::JIT.HardwareIntrinsics.Arm._Sve.Program.Sve_TrigonometricStartingValue_double_ulong() : 24
===================Running jitstress===================
------------------- {'JitMinOpts': '1'} -------------------
------------------- {'JitStress': '1'} -------------------
------------------- {'JitStress': '2'} -------------------
------------------- {'JitStress': '1', 'TieredCompilation': '1'} -------------------
------------------- {'JitStress': '2', 'TieredCompilation': '1'} -------------------
------------------- {'TailcallStress': '1'} -------------------
------------------- {'ReadyToRun': '0'} -------------------
===================Running jitstressregs===================
------------------- {'JitStressRegs': '1'} -------------------
------------------- {'JitStressRegs': '2'} -------------------
------------------- {'JitStressRegs': '3'} -------------------
------------------- {'JitStressRegs': '4'} -------------------
------------------- {'JitStressRegs': '8'} -------------------
------------------- {'JitStressRegs': '0x10'} -------------------
------------------- {'JitStressRegs': '0x80'} -------------------
------------------- {'JitStressRegs': '0x1000'} -------------------
------------------- {'JitStressRegs': '0x2000'} -------------------
===================Running jitstress2-jitstressregs===================
------------------- {'JitStress': '2', 'JitStressRegs': '1'} -------------------
------------------- {'JitStress': '2', 'JitStressRegs': '2'} -------------------
------------------- {'JitStress': '2', 'JitStressRegs': '3'} -------------------
------------------- {'JitStress': '2', 'JitStressRegs': '4'} -------------------
------------------- {'JitStress': '2', 'JitStressRegs': '8'} -------------------
------------------- {'JitStress': '2', 'JitStressRegs': '0x10'} -------------------
------------------- {'JitStress': '2', 'JitStressRegs': '0x80'} -------------------
------------------- {'JitStress': '2', 'JitStressRegs': '0x1000'} -------------------
------------------- {'JitStress': '2', 'JitStressRegs': '0x2000'} -------------------

Starting test: .\Core_Root\corerun.exe .\HardwareIntrinsics_Arm_r\HardwareIntrinsics_Arm_r.dll Sve_Trigonometric
===================Running default===================
------------------- {} -------------------
Passed test: _Sve_r::JIT.HardwareIntrinsics.Arm._Sve.Program.Sve_TrigonometricSelectCoefficient_float_uint() : 24
Passed test: _Sve_r::JIT.HardwareIntrinsics.Arm._Sve.Program.Sve_TrigonometricSelectCoefficient_double_ulong() : 24
Passed test: _Sve_r::JIT.HardwareIntrinsics.Arm._Sve.Program.Sve_TrigonometricStartingValue_float_uint() : 24
Passed test: _Sve_r::JIT.HardwareIntrinsics.Arm._Sve.Program.Sve_TrigonometricStartingValue_double_ulong() : 24
===================Running jitstress===================
------------------- {'JitMinOpts': '1'} -------------------
------------------- {'JitStress': '1'} -------------------
------------------- {'JitStress': '2'} -------------------
------------------- {'JitStress': '1', 'TieredCompilation': '1'} -------------------
------------------- {'JitStress': '2', 'TieredCompilation': '1'} -------------------
------------------- {'TailcallStress': '1'} -------------------
------------------- {'ReadyToRun': '0'} -------------------
===================Running jitstressregs===================
------------------- {'JitStressRegs': '1'} -------------------
------------------- {'JitStressRegs': '2'} -------------------
------------------- {'JitStressRegs': '3'} -------------------
------------------- {'JitStressRegs': '4'} -------------------
------------------- {'JitStressRegs': '8'} -------------------
------------------- {'JitStressRegs': '0x10'} -------------------
------------------- {'JitStressRegs': '0x80'} -------------------
------------------- {'JitStressRegs': '0x1000'} -------------------
------------------- {'JitStressRegs': '0x2000'} -------------------
===================Running jitstress2-jitstressregs===================
------------------- {'JitStress': '2', 'JitStressRegs': '1'} -------------------
------------------- {'JitStress': '2', 'JitStressRegs': '2'} -------------------
------------------- {'JitStress': '2', 'JitStressRegs': '3'} -------------------
------------------- {'JitStress': '2', 'JitStressRegs': '4'} -------------------
------------------- {'JitStress': '2', 'JitStressRegs': '8'} -------------------
------------------- {'JitStress': '2', 'JitStressRegs': '0x10'} -------------------
------------------- {'JitStress': '2', 'JitStressRegs': '0x80'} -------------------
------------------- {'JitStress': '2', 'JitStressRegs': '0x1000'} -------------------
------------------- {'JitStress': '2', 'JitStressRegs': '0x2000'} -------------------

ghost · 2024-07-10T15:20:43Z

Note regarding the new-api-needs-documentation label:

This serves as a reminder for when your PR is modifying a ref *.cs file and adding/modifying public APIs, please make sure the API implementation in the src *.cs file is documented with triple slash comments, so the PR reviewers can sign off that change.

ghost · 2024-07-10T15:20:44Z

Note regarding the new-api-needs-documentation label:

This serves as a reminder for when your PR is modifying a ref *.cs file and adding/modifying public APIs, please make sure the API implementation in the src *.cs file is documented with triple slash comments, so the PR reviewers can sign off that change.

dotnet-policy-service · 2024-07-10T15:21:14Z

Tagging subscribers to this area: @dotnet/area-system-runtime-intrinsics
See info in area-owners.md if you want to be subscribed.

kunalspathak

Added some questions around test.

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

kunalspathak · 2024-07-11T05:24:43Z

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

+            return result;
+        }
+
+        public static float TrigonometricStartingValue(float op1, uint op2)


are we sure we are taking into account all the possibilities of the operation? https://docsmirror.github.io/A64/2023-06/shared_pseudocode.html#impl-shared.FPMul.3? @SwapnilGaikwad, can we double check this one?

The logic in the helper function is correct for the valid input (-π/4 < x <= π/4). However, behaviour of the instruction may be undefined if the input falls outside the valid range.

@amanasifkhalid - for our testing purpose, we would certainly fall out of valid range. how does the API behave for them? wondering if test is robust enough to not fail for invalid values or we at least handle them?

I've verified we are generating out-of-range values, though the tests are still passing. I can constrain the inputs to this test to ensure we don't go out of range to avoid potential undefined behavior.

I've verified we are generating out-of-range values, though the tests are still passing.

that's interesting. so undefined could be that sometime they will pass and sometime they won't.

I can constrain the inputs to this test to ensure we don't go out of range to avoid potential undefined behavior.

Or probably constaint the validation for only the lanes that had valid inputs? that way we will still have coverage for invalid values, which realistically someone can pass in?

Or probably constaint the validation for only the lanes that had valid inputs? that way we will still have coverage for invalid values, which realistically someone can pass in?

That sounds like a better idea. I'll update the validation logic to do this.

amanasifkhalid · 2024-07-12T19:27:47Z

I've updated the tests to skip validation for invalid inputs. Stress tests are still passing.

kunalspathak

LGTM. Thanks!

Implement ftssel, ftsmul

3adeb81

ghost added area-System.Runtime.Intrinsics new-api-needs-documentation labels Jul 10, 2024

amanasifkhalid added the arm-sve Work related to arm64 SVE/SVE2 support label Jul 10, 2024

dotnet-policy-service bot assigned amanasifkhalid Jul 10, 2024

amanasifkhalid mentioned this pull request Jul 10, 2024

Arm64: Implement SVE APIs #99957

Closed

kunalspathak suggested changes Jul 11, 2024

View reviewed changes

amanasifkhalid added 2 commits July 12, 2024 14:53

Merge from main

17384f2

Skip test for invalid inputs

5bb50e7

amanasifkhalid mentioned this pull request Jul 12, 2024

ARM64-SVE: Add TrigonometricMultiplyAddCoefficient #104697

Merged

kunalspathak approved these changes Jul 13, 2024

View reviewed changes

kunalspathak merged commit ab46c54 into dotnet:main Jul 13, 2024

github-actions bot locked and limited conversation to collaborators Aug 13, 2024

Comments

Conversation

amanasifkhalid commented Jul 10, 2024

Uh oh!

ghost commented Jul 10, 2024

Uh oh!

ghost commented Jul 10, 2024

Uh oh!

dotnet-policy-service bot commented Jul 10, 2024

Uh oh!

kunalspathak left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

kunalspathak Jul 11, 2024

Choose a reason for hiding this comment

Uh oh!

SwapnilGaikwad Jul 11, 2024

Choose a reason for hiding this comment

Uh oh!

kunalspathak Jul 12, 2024

Choose a reason for hiding this comment

Uh oh!

amanasifkhalid Jul 12, 2024

Choose a reason for hiding this comment

Uh oh!

kunalspathak Jul 12, 2024

Choose a reason for hiding this comment

Uh oh!

amanasifkhalid Jul 12, 2024

Choose a reason for hiding this comment

Uh oh!

amanasifkhalid commented Jul 12, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

kunalspathak left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

amanasifkhalid commented Jul 12, 2024 •

edited

Loading