Repository navigation
Arm64-SVE: Implement Vector<T> HIR with SVE - #133422
snickolls-arm wants to merge 2 commits into
Conversation
Implement the Vector<T> API surface and various internal HIR transforms using SVE intrinsics, when InstructionSet_VectorT is enabled.
github-actions
Bot
added
the
area-CodeGen-coreclr
dotnet-policy-service
Bot
added
the
community-contribution
|
Azure Pipelines: Successfully started running 6 pipeline(s). 10 pipeline(s) were filtered out due to trigger conditions. There may be pipelines that require an authorized user to comment /azp run to run. |
|
Tagging subscribers to this area: @JulieLeeMSFT, @jakobbotsch |
|
@dotnet/arm64-contrib @tannergooding Please could I have a review for this patch? It fills out most of the API surface for N.B. I have left some of the more involved algorithms blank (such as recently added geometric/harmonic sequences) to improve on in future. These two in particular don't have a software fallback for |
|
cc: @dhartglassMSFT |
| msk = gtNewSimdIsNegativeNode(retType, msk, simdBaseType, simdSize); | ||
| retNode = gtNewSimdCndSelNode(retType, msk, ovf, tmpDup2, simdBaseType, simdSize); | ||
| } | ||
| #elif defined(TARGET_ARM64) |
There was a problem hiding this comment.
Is this ARM64 ifdef needed here? The previous ARM64 ifdef line ~3087 seems to do the same thing.
Similar question for SubtractSaturate below
| assert((totalSize <= 64) && (totalSize <= MaxStructSize)); | ||
|
|
||
| #if defined(TARGET_ARM64) | ||
| // The VM returns the concrete runtime contents of the field, but a scalable GT_CNS_VEC |
There was a problem hiding this comment.
do we want a TODO to look for basic sequences/constants, or do you think thats not worth it
| // | ||
| // For equality, we test whether this sequence returned zero. | ||
| LIR::Use originalUse; | ||
| BlockRange().TryGetUse(node, &originalUse); |
There was a problem hiding this comment.
TryGetUse can fail to find a use
| GenTree* vectorLength = evalVectorCount(vectorTHandle, simdBaseType); | ||
|
|
||
| GenTree* op2Clone = nullptr; | ||
| op2 = impCloneExpr(op2, &op2Clone, CHECK_SPILL_ALL, nullptr DEBUGARG("Clone index for Vector<T> bounds check")); |
There was a problem hiding this comment.
op2 may have side effects, and impCloneExpr may force a spill to a temp first. This can mess up evaluation order. The "if (rangeCheckNeeded)" along the non-scalable path below in this routine also considers that.
|
|
||
| op2 = gtNewOperNode(GT_COMMA, op2->TypeGet(), boundsCheck, op2); | ||
|
|
||
| if (op2->IsIntegralConst()) |
There was a problem hiding this comment.
Does this check ever succeed? (IsIntegralConst())
Since op2 is a GT_COMMA here
This branch has not been deployed
4 participants
Implement the
Vector<T>API surface and various internal HIR transforms using SVE intrinsics, when InstructionSet_VectorT is enabled.