Skip to content

Commit 1374fcb

Browse files
TIHanTIHan
committed
Feedback
1 parent a208c15 commit 1374fcb

File tree

7 files changed

+223
-341
lines changed

7 files changed

+223
-341
lines changed

‎src/coreclr/jit/hwintrinsic.h

+1-1
Original file line numberDiff line numberDiff line change
@@ -934,7 +934,7 @@ struct HWIntrinsicInfo
934934
}
935935

936936
// Checks if the intrinsic has an embedded mask operation and the intrinsic returns a scalar.
937-
static bool IsEmbeddedMaskForScalarResultOperation(NamedIntrinsic id)
937+
static bool IsEmbeddedMaskForScalarResult(NamedIntrinsic id)
938938
{
939939
if (IsEmbeddedMaskedOperation(id))
940940
{

‎src/coreclr/jit/hwintrinsiccodegenarm64.cpp

+14-13
Original file line numberDiff line numberDiff line change
@@ -463,6 +463,8 @@ void CodeGen::genHWIntrinsic(GenTreeHWIntrinsic* node)
463463
regNumber embMaskOp3Reg = REG_NA;
464464
regNumber falseReg = op3Reg;
465465

466+
insOpts optEmb = opt;
467+
466468
switch (intrinEmbMask.numOperands)
467469
{
468470
case 3:
@@ -505,7 +507,7 @@ void CodeGen::genHWIntrinsic(GenTreeHWIntrinsic* node)
505507
case NI_Sve_ConvertToInt32:
506508
case NI_Sve_ConvertToUInt32:
507509
{
508-
opt = intrinEmbMask.baseType == TYP_DOUBLE ? INS_OPTS_D_TO_S : INS_OPTS_SCALABLE_S;
510+
optEmb = intrinEmbMask.baseType == TYP_DOUBLE ? INS_OPTS_D_TO_S : INS_OPTS_SCALABLE_S;
509511
break;
510512
}
511513

@@ -534,7 +536,7 @@ void CodeGen::genHWIntrinsic(GenTreeHWIntrinsic* node)
534536
// If falseValue is zero, just zero out those lanes of targetReg using `movprfx`
535537
// and /Z
536538
GetEmitter()->emitIns_R_R_R(INS_sve_movprfx, emitSize, targetReg, maskReg, targetReg,
537-
opt, soptEmb);
539+
opt);
538540
}
539541
}
540542
else if (emitter::isVectorRegister(embMaskOp1Reg) && (targetReg == embMaskOp1Reg))
@@ -545,7 +547,7 @@ void CodeGen::genHWIntrinsic(GenTreeHWIntrinsic* node)
545547

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

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

563-
GetEmitter()->emitIns_R_R_R(insEmbMask, emitSize, targetReg, maskReg, embMaskOp1Reg, opt, soptEmb);
565+
GetEmitter()->emitIns_R_R_R(insEmbMask, emitSize, targetReg, maskReg, embMaskOp1Reg, optEmb,
566+
soptEmb);
564567
break;
565568
}
566569

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

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

610613
GetEmitter()->emitIns_R_R_R(insEmbMask, emitSize, targetReg, maskReg, embMaskOp2Reg,
611-
opt);
614+
optEmb);
612615
}
613616

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

626629
// Finally, perform the actual "predicated" operation so that `targetReg` is the first operand
627630
// and `embMaskOp2Reg` is the second operand.
628-
GetEmitter()->emitIns_R_R_R(insEmbMask, emitSize, targetReg, maskReg, embMaskOp2Reg, opt);
631+
GetEmitter()->emitIns_R_R_R(insEmbMask, emitSize, targetReg, maskReg, embMaskOp2Reg, optEmb);
629632
}
630633
else
631634
{
632635
// Just perform the actual "predicated" operation so that `targetReg` is the first operand
633636
// and `embMaskOp2Reg` is the second operand.
634-
GetEmitter()->emitIns_R_R_R(insEmbMask, emitSize, targetReg, maskReg, embMaskOp2Reg, opt);
637+
GetEmitter()->emitIns_R_R_R(insEmbMask, emitSize, targetReg, maskReg, embMaskOp2Reg, optEmb);
635638
}
636639

637640
break;
@@ -800,7 +803,7 @@ void CodeGen::genHWIntrinsic(GenTreeHWIntrinsic* node)
800803

801804
// Finally, perform the desired operation.
802805
GetEmitter()->emitIns_R_R_R_R(insEmbMask, emitSize, targetReg, maskReg, embMaskOp2Reg,
803-
embMaskOp3Reg, opt);
806+
embMaskOp3Reg, optEmb);
804807

805808
break;
806809
}
@@ -2139,9 +2142,7 @@ void CodeGen::genHWIntrinsic(GenTreeHWIntrinsic* node)
21392142

21402143
case NI_Sve_ExtractAfterLastScalar:
21412144
case NI_Sve_ExtractLastScalar:
2142-
assert(HWIntrinsicInfo::IsEmbeddedMaskForScalarResultOperation(intrin.id));
2143-
assert(op1Reg != REG_NA); // this is the embedded mask
2144-
assert(op2Reg != REG_NA);
2145+
assert(HWIntrinsicInfo::IsEmbeddedMaskForScalarResult(intrin.id));
21452146

21462147
if (varTypeIsFloating(node))
21472148
{

‎src/coreclr/jit/lowerarmarch.cpp

+4-2
Original file line numberDiff line numberDiff line change
@@ -1331,8 +1331,10 @@ GenTree* Lowering::LowerHWIntrinsic(GenTreeHWIntrinsic* node)
13311331
var_types simdType = Compiler::getSIMDTypeForSize(simdSize);
13321332
GenTree* trueMask = comp->gtNewSimdAllTrueMaskNode(simdBaseJitType, simdSize);
13331333

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

0 commit comments

Comments
 (0)