-
Notifications
You must be signed in to change notification settings - Fork 4.7k
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
Arm64/SVE: Implemented Scale
and Sqrt
#103663
Conversation
updating branch due to assert issue.
…when Op1 and Op2 have different types.
Note regarding the
|
Note regarding the
|
Tagging subscribers to this area: @dotnet/area-system-runtime-intrinsics |
@@ -184,11 +184,13 @@ HARDWARE_INTRINSIC(Sve, SaturatingIncrementBy32BitElementCount, | |||
HARDWARE_INTRINSIC(Sve, SaturatingIncrementBy64BitElementCount, -1, 3, true, {INS_invalid, INS_invalid, INS_invalid, INS_invalid, INS_invalid, INS_invalid, INS_sve_sqincd, INS_sve_uqincd, INS_invalid, INS_invalid}, HW_Category_SIMD, HW_Flag_Scalable|HW_Flag_HasImmediateOperand|HW_Flag_HasEnumOperand|HW_Flag_SpecialCodeGen|HW_Flag_HasScalarInputVariant|HW_Flag_SpecialImport|HW_Flag_HasRMWSemantics) | |||
HARDWARE_INTRINSIC(Sve, SaturatingIncrementBy8BitElementCount, 0, 3, false, {INS_invalid, INS_invalid, INS_invalid, INS_invalid, INS_sve_sqincb, INS_sve_uqincb, INS_sve_sqincb, INS_sve_uqincb, INS_invalid, INS_invalid}, HW_Category_Scalar, HW_Flag_Scalable|HW_Flag_HasImmediateOperand|HW_Flag_HasEnumOperand|HW_Flag_SpecialCodeGen|HW_Flag_SpecialImport|HW_Flag_HasRMWSemantics) | |||
HARDWARE_INTRINSIC(Sve, SaturatingIncrementByActiveElementCount, -1, 2, true, {INS_invalid, INS_sve_sqincp, INS_sve_sqincp, INS_sve_sqincp, INS_sve_sqincp, INS_sve_sqincp, INS_sve_sqincp, INS_sve_sqincp, INS_invalid, INS_invalid}, HW_Category_SIMD, HW_Flag_Scalable|HW_Flag_SpecialCodeGen|HW_Flag_SpecialImport|HW_Flag_BaseTypeFromSecondArg|HW_Flag_HasRMWSemantics) | |||
HARDWARE_INTRINSIC(Sve, Scale, -1, -1, false, {INS_invalid, INS_invalid, INS_invalid, INS_invalid, INS_invalid, INS_invalid, INS_invalid, INS_invalid, INS_sve_fscale, INS_sve_fscale}, HW_Category_SIMD, HW_Flag_Scalable|HW_Flag_BaseTypeFromFirstArg|HW_Flag_EmbeddedMaskedOperation|HW_Flag_LowMaskedOperation|HW_Flag_HasRMWSemantics) |
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.
Don't think we need HW_Flag_BaseTypeFromFirstArg
for this one. Can you double check?
("_UnaryOpTestTemplate.template", "SimpleVecOpTest.template", new Dictionary<string, string> { ["TemplateName"] = "Simple", ["TemplateValidationLogic"] = SimpleVecOpTest_ValidationLogic }), | ||
("_BinaryOpTestTemplate.template", "VecPairBinOpTest.template", new Dictionary<string, string> { ["TemplateName"] = "Simple", ["TemplateValidationLogic"] = VecPairBinOpTest_ValidationLogic }), | ||
("_BinaryOp_SveTestTemplate.template", "SveVecPairBinOpTest.template", new Dictionary<string, string> { ["TemplateName"] = "Simple", ["TemplateValidationLogic"] = VecPairBinOpTest_ValidationLogic }), | ||
("_BinaryOpDifferentOpTypes_SveTestTemplate.template", "SveVecPairBinOpDifferentOpTypesTest.template", new Dictionary<string, string> { ["TemplateName"] = "Simple", ["TemplateValidationLogic"] = VecPairBinOpTest_ValidationLogic }), |
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.
Any reason why we cannot use SveVecBinOpTest*.template
?
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.
The SveVecBinOpTest
is built for when the 2 arguments are of the same type. I made SveVecPairBinOpDifferentOpTypesTest
to have a flexible variation.
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.
Is it possible to tweak Op1BaseType
and Op2BaseType
and reuse SveVecBinOpTest
? You might need some casting, but wondering if it is easy to do?
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.
Can you please rerun all the test to make sure that the changes made in template works? You can just run the tests in default mode with DOTNET_TieredCompilation=0
and no need to run stress test.
@kunalspathak I did this on my SVE machine, and everything passed in both test suites. |
/ba-g timeout issues |
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.
LGTM
Contributes to #99957
Scale stress test output:
Sqrt stress test output:
cc @dotnet/arm64-contrib