Skip to content

Commit ca905a2

Browse files
BruceForstallBruceForstall
authored
Add basic support for TYP_MASK constants (dotnet#99743)
This is to support fixing JitOptRepeat (dotnet#94250). I was seeing failures in a Tensor test where `TYP_MASK` generating instructions were getting CSE'd. When OptRepeat kicks in and runs VN over the new IR, it wants to create a "zero" value for the new CSE locals. This change creates a `TYP_MASK` constant type, `simdmask_t`, like the pre-existing `simd64_t`, `simd32_t`, etc. `simdmask_t` is basically a `simd8_t` type, but with its own type. I expanded basically every place that generally handles `simd64_t` with `simdmask_t` support. This might be more than we currently need, but it seems to be a reasonable step towards making `TYP_MASK` more first-class. However, I didn't go so far as to support load/store of these constants, for example.
1 parent 9fc8ae7 commit ca905a2

File tree

10 files changed

+243
-23
lines changed

10 files changed

+243
-23
lines changed

‎src/coreclr/jit/assertionprop.cpp

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3042,6 +3042,18 @@ GenTree* Compiler::optVNBasedFoldConstExpr(BasicBlock* block, GenTree* parent, G
30423042
break;
30433043
}
30443044
break;
3045+
3046+
case TYP_MASK:
3047+
{
3048+
simdmask_t value = vnStore->ConstantValue<simdmask_t>(vnCns);
3049+
3050+
GenTreeVecCon* vecCon = gtNewVconNode(tree->TypeGet());
3051+
memcpy(&vecCon->gtSimdVal, &value, sizeof(simdmask_t));
3052+
3053+
conValTree = vecCon;
3054+
break;
3055+
}
3056+
break;
30453057
#endif // TARGET_XARCH
30463058
#endif // FEATURE_SIMD
30473059

‎src/coreclr/jit/emit.cpp

Lines changed: 21 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -8154,13 +8154,13 @@ CORINFO_FIELD_HANDLE emitter::emitFltOrDblConst(double constValue, emitAttr attr
81548154
// Return Value:
81558155
// A field handle representing the data offset to access the constant.
81568156
//
8157+
// Note:
8158+
// Access to inline data is 'abstracted' by a special type of static member
8159+
// (produced by eeFindJitDataOffs) which the emitter recognizes as being a reference
8160+
// to constant data, not a real static field.
8161+
//
81578162
CORINFO_FIELD_HANDLE emitter::emitSimd8Const(simd8_t constValue)
81588163
{
8159-
// Access to inline data is 'abstracted' by a special type of static member
8160-
// (produced by eeFindJitDataOffs) which the emitter recognizes as being a reference
8161-
// to constant data, not a real static field.
8162-
CLANG_FORMAT_COMMENT_ANCHOR;
8163-
81648164
unsigned cnsSize = 8;
81658165
unsigned cnsAlign = cnsSize;
81668166

@@ -8177,11 +8177,6 @@ CORINFO_FIELD_HANDLE emitter::emitSimd8Const(simd8_t constValue)
81778177

81788178
CORINFO_FIELD_HANDLE emitter::emitSimd16Const(simd16_t constValue)
81798179
{
8180-
// Access to inline data is 'abstracted' by a special type of static member
8181-
// (produced by eeFindJitDataOffs) which the emitter recognizes as being a reference
8182-
// to constant data, not a real static field.
8183-
CLANG_FORMAT_COMMENT_ANCHOR;
8184-
81858180
unsigned cnsSize = 16;
81868181
unsigned cnsAlign = cnsSize;
81878182

@@ -8199,11 +8194,6 @@ CORINFO_FIELD_HANDLE emitter::emitSimd16Const(simd16_t constValue)
81998194
#if defined(TARGET_XARCH)
82008195
CORINFO_FIELD_HANDLE emitter::emitSimd32Const(simd32_t constValue)
82018196
{
8202-
// Access to inline data is 'abstracted' by a special type of static member
8203-
// (produced by eeFindJitDataOffs) which the emitter recognizes as being a reference
8204-
// to constant data, not a real static field.
8205-
CLANG_FORMAT_COMMENT_ANCHOR;
8206-
82078197
unsigned cnsSize = 32;
82088198
unsigned cnsAlign = cnsSize;
82098199

@@ -8218,11 +8208,6 @@ CORINFO_FIELD_HANDLE emitter::emitSimd32Const(simd32_t constValue)
82188208

82198209
CORINFO_FIELD_HANDLE emitter::emitSimd64Const(simd64_t constValue)
82208210
{
8221-
// Access to inline data is 'abstracted' by a special type of static member
8222-
// (produced by eeFindJitDataOffs) which the emitter recognizes as being a reference
8223-
// to constant data, not a real static field.
8224-
CLANG_FORMAT_COMMENT_ANCHOR;
8225-
82268211
unsigned cnsSize = 64;
82278212
unsigned cnsAlign = cnsSize;
82288213

@@ -8234,6 +8219,22 @@ CORINFO_FIELD_HANDLE emitter::emitSimd64Const(simd64_t constValue)
82348219
UNATIVE_OFFSET cnum = emitDataConst(&constValue, cnsSize, cnsAlign, TYP_SIMD64);
82358220
return emitComp->eeFindJitDataOffs(cnum);
82368221
}
8222+
8223+
CORINFO_FIELD_HANDLE emitter::emitSimdMaskConst(simdmask_t constValue)
8224+
{
8225+
unsigned cnsSize = 8;
8226+
unsigned cnsAlign = cnsSize;
8227+
8228+
#ifdef TARGET_XARCH
8229+
if (emitComp->compCodeOpt() == Compiler::SMALL_CODE)
8230+
{
8231+
cnsAlign = dataSection::MIN_DATA_ALIGN;
8232+
}
8233+
#endif // TARGET_XARCH
8234+
8235+
UNATIVE_OFFSET cnum = emitDataConst(&constValue, cnsSize, cnsAlign, TYP_MASK);
8236+
return emitComp->eeFindJitDataOffs(cnum);
8237+
}
82378238
#endif // TARGET_XARCH
82388239
#endif // FEATURE_SIMD
82398240

‎src/coreclr/jit/emit.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2506,6 +2506,7 @@ class emitter
25062506
#if defined(TARGET_XARCH)
25072507
CORINFO_FIELD_HANDLE emitSimd32Const(simd32_t constValue);
25082508
CORINFO_FIELD_HANDLE emitSimd64Const(simd64_t constValue);
2509+
CORINFO_FIELD_HANDLE emitSimdMaskConst(simdmask_t constValue);
25092510
#endif // TARGET_XARCH
25102511
#endif // FEATURE_SIMD
25112512
regNumber emitInsBinary(instruction ins, emitAttr attr, GenTree* dst, GenTree* src);

‎src/coreclr/jit/gentree.cpp

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3406,6 +3406,13 @@ unsigned Compiler::gtHashValue(GenTree* tree)
34063406
{
34073407
#if defined(FEATURE_SIMD)
34083408
#if defined(TARGET_XARCH)
3409+
case TYP_MASK:
3410+
{
3411+
add = genTreeHashAdd(ulo32(add), vecCon->gtSimdVal.u32[1]);
3412+
add = genTreeHashAdd(ulo32(add), vecCon->gtSimdVal.u32[0]);
3413+
break;
3414+
}
3415+
34093416
case TYP_SIMD64:
34103417
{
34113418
add = genTreeHashAdd(ulo32(add), vecCon->gtSimdVal.u32[15]);
@@ -12237,6 +12244,12 @@ void Compiler::gtDispConst(GenTree* tree)
1223712244
vecCon->gtSimdVal.u64[6], vecCon->gtSimdVal.u64[7]);
1223812245
break;
1223912246
}
12247+
12248+
case TYP_MASK:
12249+
{
12250+
printf("<0x%08x, 0x%08x>", vecCon->gtSimdVal.u32[0], vecCon->gtSimdVal.u32[1]);
12251+
break;
12252+
}
1224012253
#endif // TARGET_XARCH
1224112254

1224212255
default:

‎src/coreclr/jit/gentree.h

Lines changed: 18 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -6510,8 +6510,9 @@ struct GenTreeVecCon : public GenTree
65106510
simd16_t gtSimd16Val;
65116511

65126512
#if defined(TARGET_XARCH)
6513-
simd32_t gtSimd32Val;
6514-
simd64_t gtSimd64Val;
6513+
simd32_t gtSimd32Val;
6514+
simd64_t gtSimd64Val;
6515+
simdmask_t gtSimdMaskVal;
65156516
#endif // TARGET_XARCH
65166517

65176518
simd_t gtSimdVal;
@@ -6763,6 +6764,11 @@ struct GenTreeVecCon : public GenTree
67636764
{
67646765
return gtSimd64Val.IsAllBitsSet();
67656766
}
6767+
6768+
case TYP_MASK:
6769+
{
6770+
return gtSimdMaskVal.IsAllBitsSet();
6771+
}
67666772
#endif // TARGET_XARCH
67676773
#endif // FEATURE_SIMD
67686774

@@ -6810,6 +6816,11 @@ struct GenTreeVecCon : public GenTree
68106816
{
68116817
return left->gtSimd64Val == right->gtSimd64Val;
68126818
}
6819+
6820+
case TYP_MASK:
6821+
{
6822+
return left->gtSimdMaskVal == right->gtSimdMaskVal;
6823+
}
68136824
#endif // TARGET_XARCH
68146825
#endif // FEATURE_SIMD
68156826

@@ -6850,6 +6861,11 @@ struct GenTreeVecCon : public GenTree
68506861
{
68516862
return gtSimd64Val.IsZero();
68526863
}
6864+
6865+
case TYP_MASK:
6866+
{
6867+
return gtSimdMaskVal.IsZero();
6868+
}
68536869
#endif // TARGET_XARCH
68546870
#endif // FEATURE_SIMD
68556871

‎src/coreclr/jit/instr.cpp

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -989,6 +989,13 @@ CodeGen::OperandDesc CodeGen::genOperandDesc(GenTree* op)
989989
memcpy(&constValue, &op->AsVecCon()->gtSimdVal, sizeof(simd64_t));
990990
return OperandDesc(emit->emitSimd64Const(constValue));
991991
}
992+
993+
case TYP_MASK:
994+
{
995+
simdmask_t constValue;
996+
memcpy(&constValue, &op->AsVecCon()->gtSimdVal, sizeof(simdmask_t));
997+
return OperandDesc(emit->emitSimdMaskConst(constValue));
998+
}
992999
#endif // TARGET_XARCH
9931000
#endif // FEATURE_SIMD
9941001

‎src/coreclr/jit/lowerxarch.cpp

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8760,6 +8760,8 @@ void Lowering::TryFoldCnsVecForEmbeddedBroadcast(GenTreeHWIntrinsic* parentNode,
87608760
void Lowering::TryCompressConstVecData(GenTreeStoreInd* node)
87618761
{
87628762
assert(node->Data()->IsCnsVec());
8763+
assert(node->Data()->AsVecCon()->TypeIs(TYP_SIMD32, TYP_SIMD64));
8764+
87638765
GenTreeVecCon* vecCon = node->Data()->AsVecCon();
87648766
GenTreeHWIntrinsic* broadcast = nullptr;
87658767

‎src/coreclr/jit/simd.h

Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -277,6 +277,55 @@ struct simd64_t
277277
};
278278
static_assert_no_msg(sizeof(simd64_t) == 64);
279279

280+
struct simdmask_t
281+
{
282+
union {
283+
int8_t i8[8];
284+
int16_t i16[4];
285+
int32_t i32[2];
286+
int64_t i64[1];
287+
uint8_t u8[8];
288+
uint16_t u16[4];
289+
uint32_t u32[2];
290+
uint64_t u64[1];
291+
};
292+
293+
bool operator==(const simdmask_t& other) const
294+
{
295+
return (u64[0] == other.u64[0]);
296+
}
297+
298+
bool operator!=(const simdmask_t& other) const
299+
{
300+
return !(*this == other);
301+
}
302+
303+
static simdmask_t AllBitsSet()
304+
{
305+
simdmask_t result;
306+
307+
result.u64[0] = 0xFFFFFFFFFFFFFFFF;
308+
309+
return result;
310+
}
311+
312+
bool IsAllBitsSet() const
313+
{
314+
return *this == AllBitsSet();
315+
}
316+
317+
bool IsZero() const
318+
{
319+
return *this == Zero();
320+
}
321+
322+
static simdmask_t Zero()
323+
{
324+
return {};
325+
}
326+
};
327+
static_assert_no_msg(sizeof(simdmask_t) == 8);
328+
280329
typedef simd64_t simd_t;
281330
#else
282331
typedef simd16_t simd_t;

‎src/coreclr/jit/valuenum.cpp

Lines changed: 61 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -437,6 +437,7 @@ ValueNumStore::ValueNumStore(Compiler* comp, CompAllocator alloc)
437437
#if defined(TARGET_XARCH)
438438
, m_simd32CnsMap(nullptr)
439439
, m_simd64CnsMap(nullptr)
440+
, m_simdMaskCnsMap(nullptr)
440441
#endif // TARGET_XARCH
441442
#endif // FEATURE_SIMD
442443
, m_VNFunc0Map(nullptr)
@@ -1706,6 +1707,12 @@ ValueNumStore::Chunk::Chunk(CompAllocator alloc, ValueNum* pNextBaseVN, var_type
17061707
m_defs = new (alloc) Alloc<TYP_SIMD64>::Type[ChunkSize];
17071708
break;
17081709
}
1710+
1711+
case TYP_MASK:
1712+
{
1713+
m_defs = new (alloc) Alloc<TYP_MASK>::Type[ChunkSize];
1714+
break;
1715+
}
17091716
#endif // TARGET_XARCH
17101717
#endif // FEATURE_SIMD
17111718

@@ -1870,6 +1877,11 @@ ValueNum ValueNumStore::VNForSimd64Con(simd64_t cnsVal)
18701877
{
18711878
return VnForConst(cnsVal, GetSimd64CnsMap(), TYP_SIMD64);
18721879
}
1880+
1881+
ValueNum ValueNumStore::VNForSimdMaskCon(simdmask_t cnsVal)
1882+
{
1883+
return VnForConst(cnsVal, GetSimdMaskCnsMap(), TYP_MASK);
1884+
}
18731885
#endif // TARGET_XARCH
18741886
#endif // FEATURE_SIMD
18751887

@@ -1971,6 +1983,11 @@ ValueNum ValueNumStore::VNForGenericCon(var_types typ, uint8_t* cnsVal)
19711983
READ_VALUE(simd64_t);
19721984
return VNForSimd64Con(val);
19731985
}
1986+
case TYP_MASK:
1987+
{
1988+
READ_VALUE(simdmask_t);
1989+
return VNForSimdMaskCon(val);
1990+
}
19741991
#endif // TARGET_XARCH
19751992
#endif // FEATURE_SIMD
19761993
default:
@@ -2085,6 +2102,11 @@ ValueNum ValueNumStore::VNZeroForType(var_types typ)
20852102
{
20862103
return VNForSimd64Con(simd64_t::Zero());
20872104
}
2105+
2106+
case TYP_MASK:
2107+
{
2108+
return VNForSimdMaskCon(simdmask_t::Zero());
2109+
}
20882110
#endif // TARGET_XARCH
20892111
#endif // FEATURE_SIMD
20902112

@@ -2175,6 +2197,11 @@ ValueNum ValueNumStore::VNAllBitsForType(var_types typ)
21752197
{
21762198
return VNForSimd64Con(simd64_t::AllBitsSet());
21772199
}
2200+
2201+
case TYP_MASK:
2202+
{
2203+
return VNForSimdMaskCon(simdmask_t::AllBitsSet());
2204+
}
21782205
#endif // TARGET_XARCH
21792206
#endif // FEATURE_SIMD
21802207

@@ -2296,6 +2323,13 @@ ValueNum ValueNumStore::VNOneForSimdType(var_types simdType, var_types simdBaseT
22962323
memcpy(&simd64Val, &simdVal, sizeof(simd64_t));
22972324
return VNForSimd64Con(simd64Val);
22982325
}
2326+
2327+
case TYP_MASK:
2328+
{
2329+
// '1' doesn't make sense for TYP_MASK?
2330+
// Or should it be AllBitsSet?
2331+
unreached();
2332+
}
22992333
#endif // TARGET_XARCH
23002334

23012335
default:
@@ -3741,7 +3775,7 @@ simd32_t ValueNumStore::GetConstantSimd32(ValueNum argVN)
37413775
return ConstantValue<simd32_t>(argVN);
37423776
}
37433777

3744-
// Given a simd64 constant value number return its value as a simd32.
3778+
// Given a simd64 constant value number return its value as a simd64.
37453779
//
37463780
simd64_t ValueNumStore::GetConstantSimd64(ValueNum argVN)
37473781
{
@@ -3750,6 +3784,16 @@ simd64_t ValueNumStore::GetConstantSimd64(ValueNum argVN)
37503784

37513785
return ConstantValue<simd64_t>(argVN);
37523786
}
3787+
3788+
// Given a simdmask constant value number return its value as a simdmask.
3789+
//
3790+
simdmask_t ValueNumStore::GetConstantSimdMask(ValueNum argVN)
3791+
{
3792+
assert(IsVNConstant(argVN));
3793+
assert(TypeOfVN(argVN) == TYP_MASK);
3794+
3795+
return ConstantValue<simdmask_t>(argVN);
3796+
}
37533797
#endif // TARGET_XARCH
37543798
#endif // FEATURE_SIMD
37553799

@@ -9221,6 +9265,13 @@ void ValueNumStore::vnDump(Compiler* comp, ValueNum vn, bool isPtr)
92219265
cnsVal.u64[6], cnsVal.u64[7]);
92229266
break;
92239267
}
9268+
9269+
case TYP_MASK:
9270+
{
9271+
simdmask_t cnsVal = GetConstantSimdMask(vn);
9272+
printf("SimdMaskCns[0x%08x, 0x%08x]", cnsVal.u32[0], cnsVal.u32[1]);
9273+
break;
9274+
}
92249275
#endif // TARGET_XARCH
92259276
#endif // FEATURE_SIMD
92269277

@@ -10661,6 +10712,15 @@ void Compiler::fgValueNumberTreeConst(GenTree* tree)
1066110712
tree->gtVNPair.SetBoth(vnStore->VNForSimd64Con(simd64Val));
1066210713
break;
1066310714
}
10715+
10716+
case TYP_MASK:
10717+
{
10718+
simdmask_t simdmaskVal;
10719+
memcpy(&simdmaskVal, &tree->AsVecCon()->gtSimdVal, sizeof(simdmask_t));
10720+
10721+
tree->gtVNPair.SetBoth(vnStore->VNForSimdMaskCon(simdmaskVal));
10722+
break;
10723+
}
1066410724
#endif // TARGET_XARCH
1066510725
#endif // FEATURE_SIMD
1066610726

0 commit comments

Comments
 (0)