Add SVE2 API skeleton and implement BitwiseClearXor#115428
Add SVE2 API skeleton and implement BitwiseClearXor#115428kunalspathak merged 2 commits intodotnet:mainfrom
Conversation
|
Tagging subscribers to this area: @dotnet/area-system-runtime-intrinsics |
| [Intrinsic] | ||
| [CLSCompliant(false)] | ||
| [Experimental(Experimentals.ArmSveDiagId, UrlFormat = Experimentals.SharedUrlFormat)] | ||
| public abstract class Sve2 : Sve |
There was a problem hiding this comment.
Just need to confirm that this inheritance matches the agreed design, as there was some discrepancy in our code generation scripts? I assumed it should be like this, as the architecture has this dependency and some of the API design issues had this inheritance.
There was a problem hiding this comment.
This makes sense to me.
| } | ||
|
|
||
| case NI_Sve2_BitwiseClearXor: | ||
| assert(targetReg == op1Reg); |
There was a problem hiding this comment.
you need to add movprfx if targetReg != op1Reg something like:
if (targetReg != op1Reg)
{
assert(targetReg != op2Reg);
GetEmitter()->emitInsSve_R_R(INS_sve_movprfx, EA_SCALABLE, targetReg, op1Reg);
}See how we handle TrigonometricMultiplyAddCoefficient in this file.
There was a problem hiding this comment.
And it seems that's why we are seeing test failure too:
Assert failure(PID 60 [0x0000003c], Thread: 60 [0x003c]): Assertion failed 'targetReg == op1Reg' in 'JIT.HardwareIntrinsics.Arm._Sve2.SimpleTernaryOpTest__Sve2_BitwiseClearXor_sbyte:RunLclVarScenario_UnsafeRead():this' during 'Generate code' (IL size 113; hash 0xa6f94412; FullOpts)
File: /__w/1/s/src/coreclr/jit/hwintrinsiccodegenarm64.cpp:2653
Image: /root/helix/work/correlation/corerun
There was a problem hiding this comment.
The need for movprfx for RMW handling is notably why x64 has helpers like emitIns_SIMD_R_R_R: https://github.com/dotnet/runtime/blob/main/src/coreclr/jit/emitxarch.cpp#L9766-L9791
Such helpers allow codegen to mostly ignore the concept and just thing in terms of number of input/outputs instead. It allows for emit to centralize the handling instead. This makes it much simpler to update if we have new considerations in the future, to avoid accidentally missing the handling in various locations, etc.
There was a problem hiding this comment.
I agree. The movprfx logic needs to be centralized for sure.
| using System; | ||
| using System.Collections.Generic; | ||
|
|
||
| namespace JIT.HardwareIntrinsics.Arm._Sve2 |
There was a problem hiding this comment.
I am wondering if there is a need to create Sve2 folder and new project for it or just use existing Sve folder and projects?
There was a problem hiding this comment.
It's slightly better for running the tests while developing, as it's faster to run the SVE2 suite on it's own and easier to filter down.
This is the first of the SVE2 intrinsics, laying out the test framework and adding the SVE2 API class.
@a74nh @kunalspathak