Skip to content

Commit

Permalink
Feedback
Browse files Browse the repository at this point in the history
  • Loading branch information
@TIHan
TIHan committed Jun 26, 2024
1 parent a208c15 commit 1374fcb
Show file tree
Hide file tree
Showing 7 changed files with 223 additions and 341 deletions.
2 changes: 1 addition & 1 deletion src/coreclr/jit/hwintrinsic.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -934,7 +934,7 @@ struct HWIntrinsicInfo
}

// Checks if the intrinsic has an embedded mask operation and the intrinsic returns a scalar.
static bool IsEmbeddedMaskForScalarResultOperation(NamedIntrinsic id)
static bool IsEmbeddedMaskForScalarResult(NamedIntrinsic id)
{
if (IsEmbeddedMaskedOperation(id))
{
Expand Down
27 changes: 14 additions & 13 deletions src/coreclr/jit/hwintrinsiccodegenarm64.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -463,6 +463,8 @@ void CodeGen::genHWIntrinsic(GenTreeHWIntrinsic* node)
regNumber embMaskOp3Reg = REG_NA;
regNumber falseReg = op3Reg;

insOpts optEmb = opt;

This comment has been minimized.

Sign in to view

Copy link
@kunalspathak

kunalspathak Jun 26, 2024

Member

this can be inside below switch case 1: and just use it for case 1:. No need to use optEmb in other cases.

switch (intrinEmbMask.numOperands)
{
case 3:
Expand Down Expand Up @@ -505,7 +507,7 @@ void CodeGen::genHWIntrinsic(GenTreeHWIntrinsic* node)
case NI_Sve_ConvertToInt32:
case NI_Sve_ConvertToUInt32:
{
opt = intrinEmbMask.baseType == TYP_DOUBLE ? INS_OPTS_D_TO_S : INS_OPTS_SCALABLE_S;
optEmb = intrinEmbMask.baseType == TYP_DOUBLE ? INS_OPTS_D_TO_S : INS_OPTS_SCALABLE_S;
break;
}

Expand Down Expand Up @@ -534,7 +536,7 @@ void CodeGen::genHWIntrinsic(GenTreeHWIntrinsic* node)
// If falseValue is zero, just zero out those lanes of targetReg using `movprfx`
// and /Z
GetEmitter()->emitIns_R_R_R(INS_sve_movprfx, emitSize, targetReg, maskReg, targetReg,
opt, soptEmb);
opt);
}
}
else if (emitter::isVectorRegister(embMaskOp1Reg) && (targetReg == embMaskOp1Reg))
Expand All @@ -545,7 +547,7 @@ void CodeGen::genHWIntrinsic(GenTreeHWIntrinsic* node)

// We cannot use use `movprfx` here to move falseReg to targetReg because that will
// overwrite the value of embMaskOp1Reg which is present in targetReg.
GetEmitter()->emitIns_R_R_R(insEmbMask, emitSize, targetReg, maskReg, embMaskOp1Reg, opt,
GetEmitter()->emitIns_R_R_R(insEmbMask, emitSize, targetReg, maskReg, embMaskOp1Reg, optEmb,
soptEmb);

GetEmitter()->emitIns_R_R_R_R(INS_sve_sel, emitSize, targetReg, maskReg, targetReg,
Expand All @@ -560,7 +562,8 @@ void CodeGen::genHWIntrinsic(GenTreeHWIntrinsic* node)
}
}

GetEmitter()->emitIns_R_R_R(insEmbMask, emitSize, targetReg, maskReg, embMaskOp1Reg, opt, soptEmb);
GetEmitter()->emitIns_R_R_R(insEmbMask, emitSize, targetReg, maskReg, embMaskOp1Reg, optEmb,
soptEmb);
break;
}

Expand All @@ -578,7 +581,7 @@ void CodeGen::genHWIntrinsic(GenTreeHWIntrinsic* node)

// Finally, perform the actual "predicated" operation so that `targetReg` is the first operand
// and `embMaskOp2Reg` is the second operand.
GetEmitter()->emitIns_R_R_R(insEmbMask, emitSize, targetReg, maskReg, embMaskOp2Reg, opt);
GetEmitter()->emitIns_R_R_R(insEmbMask, emitSize, targetReg, maskReg, embMaskOp2Reg, optEmb);
}
else if (targetReg != falseReg)
{
Expand All @@ -593,7 +596,7 @@ void CodeGen::genHWIntrinsic(GenTreeHWIntrinsic* node)
// If the embedded instruction supports optional mask operation, use the "unpredicated"
// version of the instruction, followed by "sel" to select the active lanes.
GetEmitter()->emitIns_R_R_R(insEmbMask, emitSize, targetReg, embMaskOp1Reg,
embMaskOp2Reg, opt);
embMaskOp2Reg, optEmb);
}
else
{
Expand All @@ -608,7 +611,7 @@ void CodeGen::genHWIntrinsic(GenTreeHWIntrinsic* node)
GetEmitter()->emitIns_R_R(INS_sve_movprfx, EA_SCALABLE, targetReg, embMaskOp1Reg);

GetEmitter()->emitIns_R_R_R(insEmbMask, emitSize, targetReg, maskReg, embMaskOp2Reg,
opt);
optEmb);
}

GetEmitter()->emitIns_R_R_R_R(INS_sve_sel, emitSize, targetReg, maskReg, targetReg,
Expand All @@ -625,13 +628,13 @@ void CodeGen::genHWIntrinsic(GenTreeHWIntrinsic* node)

// Finally, perform the actual "predicated" operation so that `targetReg` is the first operand
// and `embMaskOp2Reg` is the second operand.
GetEmitter()->emitIns_R_R_R(insEmbMask, emitSize, targetReg, maskReg, embMaskOp2Reg, opt);
GetEmitter()->emitIns_R_R_R(insEmbMask, emitSize, targetReg, maskReg, embMaskOp2Reg, optEmb);
}
else
{
// Just perform the actual "predicated" operation so that `targetReg` is the first operand
// and `embMaskOp2Reg` is the second operand.
GetEmitter()->emitIns_R_R_R(insEmbMask, emitSize, targetReg, maskReg, embMaskOp2Reg, opt);
GetEmitter()->emitIns_R_R_R(insEmbMask, emitSize, targetReg, maskReg, embMaskOp2Reg, optEmb);
}

break;
Expand Down Expand Up @@ -800,7 +803,7 @@ void CodeGen::genHWIntrinsic(GenTreeHWIntrinsic* node)

// Finally, perform the desired operation.
GetEmitter()->emitIns_R_R_R_R(insEmbMask, emitSize, targetReg, maskReg, embMaskOp2Reg,
embMaskOp3Reg, opt);
embMaskOp3Reg, optEmb);

break;
}
Expand Down Expand Up @@ -2139,9 +2142,7 @@ void CodeGen::genHWIntrinsic(GenTreeHWIntrinsic* node)

case NI_Sve_ExtractAfterLastScalar:
case NI_Sve_ExtractLastScalar:
assert(HWIntrinsicInfo::IsEmbeddedMaskForScalarResultOperation(intrin.id));
assert(op1Reg != REG_NA); // this is the embedded mask
assert(op2Reg != REG_NA);
assert(HWIntrinsicInfo::IsEmbeddedMaskForScalarResult(intrin.id));

if (varTypeIsFloating(node))
{
Expand Down
6 changes: 4 additions & 2 deletions src/coreclr/jit/lowerarmarch.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1331,8 +1331,10 @@ GenTree* Lowering::LowerHWIntrinsic(GenTreeHWIntrinsic* node)
var_types simdType = Compiler::getSIMDTypeForSize(simdSize);
GenTree* trueMask = comp->gtNewSimdAllTrueMaskNode(simdBaseJitType, simdSize);


if (HWIntrinsicInfo::IsEmbeddedMaskForScalarResultOperation(intrinsicId))
// The instruction uses "predicate" to pick lanes, but at the same time returns a scalar result.
// As such, we cannot wrap it inside ConditionalSelect because that node operates on TYP_SIMD.
// Hence, we will just add an operand so that we have a predicate register for such instructions.
if (HWIntrinsicInfo::IsEmbeddedMaskForScalarResult(intrinsicId))
{
// Create the same node with an additional operand to pass the mask.
GenTreeHWIntrinsic* newNode =
Expand Down
Loading

0 comments on commit 1374fcb

Please sign in to comment.