SVE: Added Load2xVectorAndUnzip, Load3xVectorAndUnzip, Load4xVectorAndUnzip APIs
#102180
Conversation
This file contains 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
|
Note regarding the |
|
Tagging subscribers to this area: @dotnet/area-system-runtime-intrinsics |
kunalspathak
requested changes
May 14, 2024
There was a problem hiding this comment.
can you also follow the instructions at #99957 (comment) and confirm if all the newly added tests pass? I can work with you on machine where you can run these tests.
src/tests/JIT/HardwareIntrinsics/Arm/Shared/SveLoadVectorx2Test.template
Outdated
Show resolved
Hide resolved
src/libraries/System.Private.CoreLib/src/System/Runtime/Intrinsics/Arm/Sve.cs
Show resolved
Hide resolved
src/tests/Common/GenerateHWIntrinsicTests/GenerateHWIntrinsicTests_Arm.cs
Outdated
Show resolved
Hide resolved
|
The tests don't pass right now, but I think the fix your suggesting will work. |
a74nh
reviewed
May 14, 2024
src/libraries/System.Private.CoreLib/src/System/Runtime/Intrinsics/Arm/Sve.cs
Outdated
Show resolved
Hide resolved
|
Tests pass now: |
It is running just the |
|
Gotcha. Will need to find an arm64 machine that supports it then. |
|
@dotnet/arm64-contrib @kunalspathak this is ready. Had to play whack-a-mole. Tests are passing and running :) |
a74nh
reviewed
May 16, 2024
kunalspathak
requested changes
May 16, 2024
src/coreclr/jit/lsraarm64.cpp
Outdated
| @@ -1758,6 +1758,18 @@ int LinearScan::BuildHWIntrinsic(GenTreeHWIntrinsic* intrinsicTree, int* pDstCou | |||
| break; | |||
| } | |||
|
|
|||
| case NI_Sve_LoadVectorx2: | |||
There was a problem hiding this comment.
this can be combined with above cases:
case NI_Sve_LoadVectorx2:
case NI_Sve_LoadVectorx3:
case NI_Sve_LoadVectorx4:
assert(intrin.op2 != nullptr);
srcCount += BuildOperandUses(intrin.op2);
FALLTHROUGH;
case NI_AdvSimd_LoadVector64x2AndUnzip:
case NI_AdvSimd_LoadVector64x3AndUnzip:
case NI_AdvSimd_LoadVector64x4AndUnzip:
case NI_AdvSimd_Arm64_LoadVector128x2AndUnzip:
case NI_AdvSimd_Arm64_LoadVector128x3AndUnzip:
case NI_AdvSimd_Arm64_LoadVector128x4AndUnzip:
case NI_AdvSimd_LoadVector64x2:
case NI_AdvSimd_LoadVector64x3:
case NI_AdvSimd_LoadVector64x4:
case NI_AdvSimd_Arm64_LoadVector128x2:
case NI_AdvSimd_Arm64_LoadVector128x3:
case NI_AdvSimd_Arm64_LoadVector128x4:
case NI_AdvSimd_LoadAndReplicateToVector64x2:
case NI_AdvSimd_LoadAndReplicateToVector64x3:
case NI_AdvSimd_LoadAndReplicateToVector64x4:
case NI_AdvSimd_Arm64_LoadAndReplicateToVector128x2:
case NI_AdvSimd_Arm64_LoadAndReplicateToVector128x3:
case NI_AdvSimd_Arm64_LoadAndReplicateToVector128x4:
{
assert(intrin.op1 != nullptr);
BuildConsecutiveRegistersForDef(intrinsicTree, dstCount);
*pDstCount = dstCount;
break;
}There was a problem hiding this comment.
This is still not resolved.
There was a problem hiding this comment.
I can't do this as a fallthrough because there is another fallthrough above it.
src/tests/JIT/HardwareIntrinsics/Arm/Shared/SveLoadVectorx2Test.template
Outdated
Show resolved
Hide resolved
src/tests/Common/GenerateHWIntrinsicTests/GenerateHWIntrinsicTests_Arm.cs
Outdated
Show resolved
Hide resolved
TIHan
changed the title
LoadVectorx2, LoadVectorx3, LoadVectorx4 APIsLoad2xVectorAndUnzip, Load3xVectorAndUnzip, Load4xVectorAndUnzip APIs
|
Stress modes are failing badly for the |
This was referenced May 21, 2024
|
Having some failures with Load2xVector and Load3xVector in stress testing; the results are coming back as zeroes for public void RunClassFldScenario()
{
TestLibrary.TestFramework.BeginScenario(nameof(RunClassFldScenario));
//TODO-SVE: Once register allocation exists for predicates, move loadMask into DataTable
{Op1VectorType}<{Op1BaseType}> loadMask = Sve.CreateTrueMask{RetBaseType}(SveMaskPattern.All);
(_fld1, _fld2) = {Isa}.{Method}(loadMask, ({Op1BaseType}*)_dataTable.inArrayPtr);
ValidateResult(_fld1, _fld2, _dataTable.inArrayPtr);
} |
|
@kunalspathak this is ready. I do get an assertion in the non_faulting cases under stress modes, but it appears to be related to Where the assertion appears: case IF_SVE_ID_2A: // ..........iiiiii ...iiinnnnn.TTTT -- SVE load predicate register
case IF_SVE_JG_2A: // ..........iiiiii ...iiinnnnn.TTTT -- SVE store predicate register
assert(insOptsNone(id->idInsOpt()));
assert(isScalableVectorSize(id->idOpSize()));
assert(isPredicateRegister(id->idReg1())); // TTTT
assert(isGeneralRegisterOrZR(id->idReg2())); // nnnnn
assert(isValidSimm<9>(emitGetInsSC(id))); // iii <------ ASSERTION FAILS
break; |
kunalspathak
requested changes
May 26, 2024
src/tests/JIT/HardwareIntrinsics/Arm/Shared/SveLoad2xVectorAndUnzipTest.template
Outdated
Show resolved
Hide resolved
|
@kunalspathak this is ready again. |
Ruihan-Yin
pushed a commit
to Ruihan-Yin/runtime
that referenced
this pull request
May 30, 2024
…torAndUnzip` APIs (dotnet#102180)
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
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.
Contributes to #99957
Adds:
Load2xVectorAndUnzipLoad3xVectorAndUnzipLoad4xVectorAndUnzip