Skip to content

Commit 07f788a

Browse files
tannergoodingtannergooding
committed
Ensure that embedded mask handling is covering the right scenarios
1 parent fa1b389 commit 07f788a

File tree

7 files changed

+219
-141
lines changed

7 files changed

+219
-141
lines changed

‎src/coreclr/jit/codegeninterface.h

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -184,6 +184,8 @@ class CodeGenInterface
184184

185185
static unsigned instInputSize(instruction ins);
186186
static unsigned instKMaskBaseSize(instruction ins);
187+
188+
bool IsEmbeddedBroadcastEnabled(instruction ins, GenTree* op);
187189
#endif // TARGET_XARCH
188190
//-------------------------------------------------------------------------
189191
// Liveness-related fields & methods
@@ -825,10 +827,6 @@ class CodeGenInterface
825827

826828
virtual const char* siStackVarName(size_t offs, size_t size, unsigned reg, unsigned stkOffs) = 0;
827829
#endif // LATE_DISASM
828-
829-
#if defined(TARGET_XARCH)
830-
bool IsEmbeddedBroadcastEnabled(instruction ins, GenTree* op);
831-
#endif
832830
};
833831

834832
#if !defined(TARGET_LOONGARCH64) && !defined(TARGET_RISCV64)

‎src/coreclr/jit/emitxarch.cpp

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -2205,11 +2205,11 @@ emitter::code_t emitter::AddEvexPrefix(const instrDesc* id, code_t code, emitAtt
22052205
if (aaaContext != 0)
22062206
{
22072207
maskReg = static_cast<regNumber>(aaaContext + KBASE);
2208+
}
22082209

2209-
if (id->idIsEvexZContextSet())
2210-
{
2211-
code |= ZBIT_IN_BYTE_EVEX_PREFIX;
2212-
}
2210+
if (id->idIsEvexZContextSet())
2211+
{
2212+
code |= ZBIT_IN_BYTE_EVEX_PREFIX;
22132213
}
22142214
break;
22152215
}

‎src/coreclr/jit/emitxarch.h

Lines changed: 5 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -523,15 +523,12 @@ void SetEvexEmbMaskIfNeeded(instrDesc* id, insOpts instOptions)
523523
{
524524
assert(UseEvexEncoding());
525525
id->idSetEvexAaaContext(instOptions);
526-
527-
if ((instOptions & INS_OPTS_EVEX_z_MASK) == INS_OPTS_EVEX_em_zero)
528-
{
529-
id->idSetEvexZContext();
530-
}
531526
}
532-
else
527+
528+
if ((instOptions & INS_OPTS_EVEX_z_MASK) == INS_OPTS_EVEX_em_zero)
533529
{
534-
assert((instOptions & INS_OPTS_EVEX_z_MASK) == 0);
530+
assert(UseEvexEncoding());
531+
id->idSetEvexZContext();
535532
}
536533
}
537534

@@ -1290,7 +1287,7 @@ inline bool HasEmbeddedBroadcast(const instrDesc* id) const
12901287
//
12911288
inline bool HasEmbeddedMask(const instrDesc* id) const
12921289
{
1293-
return id->idIsEvexAaaContextSet();
1290+
return id->idIsEvexAaaContextSet() || id->idIsEvexZContextSet();
12941291
}
12951292

12961293
inline bool HasHighSIMDReg(const instrDesc* id) const;

‎src/coreclr/jit/hwintrinsic.h

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -557,6 +557,25 @@ struct HWIntrinsicInfo
557557
FloatComparisonMode comparison,
558558
var_types simdBaseType,
559559
unsigned simdSize);
560+
561+
//------------------------------------------------------------------------
562+
// genIsTableDrivenHWIntrinsic:
563+
//
564+
// Arguments:
565+
// category - category of a HW intrinsic
566+
//
567+
// Return Value:
568+
// returns true if this category can be table-driven in CodeGen
569+
//
570+
static bool genIsTableDrivenHWIntrinsic(NamedIntrinsic intrinsicId, HWIntrinsicCategory category)
571+
{
572+
// TODO - make more categories to the table-driven framework
573+
// HW_Category_Helper and HW_Flag_SpecialCodeGen usually need manual codegen
574+
const bool tableDrivenCategory =
575+
(category != HW_Category_Special) && (category != HW_Category_Scalar) && (category != HW_Category_Helper);
576+
const bool tableDrivenFlag = !HWIntrinsicInfo::HasSpecialCodegen(intrinsicId);
577+
return tableDrivenCategory && tableDrivenFlag;
578+
}
560579
#endif
561580

562581
// Member lookup

‎src/coreclr/jit/hwintrinsiccodegenxarch.cpp

Lines changed: 20 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -62,25 +62,6 @@ static void assertIsContainableHWIntrinsicOp(Lowering* lowering,
6262
#endif // DEBUG
6363
}
6464

65-
//------------------------------------------------------------------------
66-
// genIsTableDrivenHWIntrinsic:
67-
//
68-
// Arguments:
69-
// category - category of a HW intrinsic
70-
//
71-
// Return Value:
72-
// returns true if this category can be table-driven in CodeGen
73-
//
74-
static bool genIsTableDrivenHWIntrinsic(NamedIntrinsic intrinsicId, HWIntrinsicCategory category)
75-
{
76-
// TODO - make more categories to the table-driven framework
77-
// HW_Category_Helper and HW_Flag_SpecialCodeGen usually need manual codegen
78-
const bool tableDrivenCategory =
79-
(category != HW_Category_Special) && (category != HW_Category_Scalar) && (category != HW_Category_Helper);
80-
const bool tableDrivenFlag = !HWIntrinsicInfo::HasSpecialCodegen(intrinsicId);
81-
return tableDrivenCategory && tableDrivenFlag;
82-
}
83-
8465
//------------------------------------------------------------------------
8566
// AddEmbRoundingMode: Adds the embedded rounding mode to the insOpts
8667
//
@@ -408,7 +389,7 @@ void CodeGen::genHWIntrinsic(GenTreeHWIntrinsic* node)
408389
assert(HWIntrinsicInfo::RequiresCodegen(intrinsicId));
409390
assert(!HWIntrinsicInfo::NeedsNormalizeSmallTypeToInt(intrinsicId) || !varTypeIsSmall(node->GetSimdBaseType()));
410391

411-
bool isTableDriven = genIsTableDrivenHWIntrinsic(intrinsicId, category);
392+
bool isTableDriven = HWIntrinsicInfo::genIsTableDrivenHWIntrinsic(intrinsicId, category);
412393
insOpts instOptions = INS_OPTS_NONE;
413394

414395
if (GetEmitter()->UseEvexEncoding())
@@ -659,6 +640,8 @@ void CodeGen::genHWIntrinsic(GenTreeHWIntrinsic* node)
659640
{
660641
// Until we improve the handling of addressing modes in the emitter, we'll create a
661642
// temporary GT_IND to generate code with.
643+
644+
assert(instOptions == INS_OPTS_NONE);
662645
GenTreeIndir load = indirForm(node->TypeGet(), op1);
663646
emit->emitInsLoadInd(ins, simdSize, node->GetRegNum(), &load);
664647
}
@@ -703,6 +686,7 @@ void CodeGen::genHWIntrinsic(GenTreeHWIntrinsic* node)
703686
// Until we improve the handling of addressing modes in the emitter, we'll create a
704687
// temporary GT_STORE_IND to generate code with.
705688

689+
assert(instOptions == INS_OPTS_NONE);
706690
GenTreeStoreInd store = storeIndirForm(node->TypeGet(), op1, op2);
707691
emit->emitInsStoreInd(ins, simdSize, &store);
708692
break;
@@ -834,10 +818,9 @@ void CodeGen::genHWIntrinsic(GenTreeHWIntrinsic* node)
834818
}
835819
else if (category == HW_Category_MemoryStore)
836820
{
837-
assert(instOptions == INS_OPTS_NONE);
838-
839821
// The Mask instructions do not currently support containment of the address.
840822
assert(!op2->isContained());
823+
841824
if (intrinsicId == NI_AVX_MaskStore || intrinsicId == NI_AVX2_MaskStore)
842825
{
843826
emit->emitIns_AR_R_R(ins, simdSize, op2Reg, op3Reg, op1Reg, 0, instOptions);
@@ -1340,9 +1323,22 @@ void CodeGen::genHWIntrinsic_R_R_RM_R(GenTreeHWIntrinsic* node, instruction ins,
13401323
GenTree* op3 = node->Op(3);
13411324
emitter* emit = GetEmitter();
13421325

1343-
regNumber op1Reg = op1->GetRegNum();
1326+
regNumber op1Reg = REG_NA;
13441327
regNumber op3Reg = op3->GetRegNum();
13451328

1329+
if (op1->isContained())
1330+
{
1331+
assert(node->GetHWIntrinsicId() == NI_AVX512_BlendVariableMask);
1332+
assert(op1->IsVectorZero());
1333+
1334+
instOptions = AddEmbMaskingMode(instOptions, REG_K0, true);
1335+
op1Reg = targetReg;
1336+
}
1337+
else
1338+
{
1339+
op1Reg = op1->GetRegNum();
1340+
}
1341+
13461342
assert(targetReg != REG_NA);
13471343
assert(op1Reg != REG_NA);
13481344
assert(op3Reg != REG_NA);
@@ -1693,7 +1689,7 @@ void CodeGen::genNonTableDrivenHWIntrinsicsJumpTableFallback(GenTreeHWIntrinsic*
16931689

16941690
assert(HWIntrinsicInfo::IsEmbRoundingCompatible(intrinsicId));
16951691
assert(!lastOp->isContained());
1696-
assert(!genIsTableDrivenHWIntrinsic(intrinsicId, category));
1692+
assert(!HWIntrinsicInfo::genIsTableDrivenHWIntrinsic(intrinsicId, category));
16971693

16981694
var_types baseType = node->GetSimdBaseType();
16991695
emitAttr attr = emitActualTypeSize(Compiler::getSIMDTypeForSize(node->GetSimdSize()));

0 commit comments

Comments
 (0)