-
Notifications
You must be signed in to change notification settings - Fork 5.2k
Implement SVE2 ConvertToSingleOdd and ConvertToSingleRoundToOdd #118957
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?
Changes from all commits
8644798
7be8db6
3eb0933
88e0f99
f76ad91
50d31ff
a25d143
87b68d2
423fac9
5c4bcbd
77bcdf7
ee0c54e
30748fc
550c4b2
ae43aae
98603ec
7b56164
fa11f81
9e1ffe5
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -722,6 +722,11 @@ void CodeGen::genHWIntrinsic(GenTreeHWIntrinsic* node) | |
assert(intrin.op3->IsVectorZero()); | ||
break; | ||
|
||
case NI_Sve2_ConvertToSingleOdd: | ||
case NI_Sve2_ConvertToSingleOddRoundToOdd: | ||
embOpt = INS_OPTS_D_TO_S; | ||
break; | ||
|
||
default: | ||
break; | ||
} | ||
|
@@ -798,6 +803,16 @@ void CodeGen::genHWIntrinsic(GenTreeHWIntrinsic* node) | |
falseReg, opt); | ||
break; | ||
|
||
case NI_Sve2_ConvertToSingleOdd: | ||
case NI_Sve2_ConvertToSingleOddRoundToOdd: | ||
// TODO-SVE: Optimise away the explicit copying of `embMaskOp1Reg` to `targetReg`. | ||
// For these intrinsics we cannot use movprfx instruction to populate `targetReg` with | ||
// `embMaskOp1Reg`. Thus, we need to perform move before the operation. | ||
GetEmitter()->emitIns_Mov(INS_sve_mov, emitSize, targetReg, embMaskOp1Reg, | ||
/* canSkip */ true, INS_OPTS_SCALABLE_S); | ||
emitInsHelper(targetReg, maskReg, embMaskOp2Reg); | ||
break; | ||
|
||
default: | ||
assert(targetReg != embMaskOp2Reg); | ||
|
||
|
@@ -825,12 +840,25 @@ void CodeGen::genHWIntrinsic(GenTreeHWIntrinsic* node) | |
break; | ||
} | ||
} | ||
// If `targetReg` and `falseReg` are not same, then we need to move it to `targetReg` first | ||
// so the `insEmbMask` operation can be merged on top of it. | ||
else if (targetReg != falseReg) | ||
{ | ||
// If `targetReg` and `falseReg` are not same, then we need to move it to `targetReg` first | ||
// so the `insEmbMask` operation can be merged on top of it. | ||
|
||
if (falseReg != embMaskOp1Reg) | ||
if ((intrinEmbMask.id == NI_Sve2_ConvertToSingleOdd) || | ||
(intrinEmbMask.id == NI_Sve2_ConvertToSingleOddRoundToOdd)) | ||
{ | ||
// TODO-SVE: Optimise away the explicit copying of `embMaskOp1Reg` to `targetReg`. | ||
// For these intrinsics we cannot use movprfx instruction to populate `targetReg` with | ||
// `embMaskOp1Reg`. Thus, we need to perform move before the operation, and then "sel" to | ||
// select the active lanes. | ||
GetEmitter()->emitIns_Mov(INS_sve_mov, emitSize, targetReg, embMaskOp1Reg, | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. we have some asserts in other places that asserts we aren't overwriting one of the input registers. Do we need such asserts here as well (i.e. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'll take a look. I didn't understand fully how an assert would help here. For There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It's RMW and so overwriting one of the inputs is expected. The consideration is ensuring we're not overwriting the other input. That is, we have a requirement that However, if we didn't mark The assert is meant to catch this scenario and ensure we correctly marked it as delay free in LSRA. We'd want similar assertions for other correctness considerations (not that these are needed here, just generally speaking), like the typical 3 cases of:
|
||
/* canSkip */ true, INS_OPTS_SCALABLE_S); | ||
emitInsHelper(targetReg, maskReg, embMaskOp2Reg); | ||
GetEmitter()->emitIns_R_R_R_R(INS_sve_sel, emitSize, targetReg, maskReg, targetReg, | ||
falseReg, opt); | ||
} | ||
else if (falseReg != embMaskOp1Reg) | ||
{ | ||
// At the point, targetReg != embMaskOp1Reg != falseReg | ||
if (HWIntrinsicInfo::IsOptionalEmbeddedMaskedOperation(intrinEmbMask.id)) | ||
|
Uh oh!
There was an error while loading. Please reload this page.