-
Notifications
You must be signed in to change notification settings - Fork 5k
Add SVE2 API skeleton and implement BitwiseClearXor #115428
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This makes sense to me.
@@ -2649,6 +2649,12 @@ void CodeGen::genHWIntrinsic(GenTreeHWIntrinsic* node) | |||
break; | |||
} | |||
|
|||
case NI_Sve2_BitwiseClearXor: | |||
assert(targetReg == op1Reg); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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
using System; | ||
using System.Collections.Generic; | ||
|
||
namespace JIT.HardwareIntrinsics.Arm._Sve2 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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