Skip to content

Commit 4a0eba9

Browse files
tannergoodingtannergooding
committed
Add some basic asserts to ensure _idCustom# isn't used incorrectly
1 parent 1532212 commit 4a0eba9

File tree

5 files changed

+39
-27
lines changed

5 files changed

+39
-27
lines changed

‎src/coreclr/jit/emit.h‎

Lines changed: 11 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -779,9 +779,7 @@ class emitter
779779
#define _idBound _idCustom1 /* jump target / frame offset bound */
780780
#define _idTlsGD _idCustom2 /* Used to store information related to TLS GD access on linux */
781781
#define _idNoGC _idCustom3 /* Some helpers don't get recorded in GC tables */
782-
#define _idEvexAaaContext \
783-
((_idCustom3 << 2) | (_idCustom2 << 1) | _idCustom1) /* bits used for the EVEX.aaa context \
784-
*/
782+
#define _idEvexAaaContext (_idCustom3 << 2) | (_idCustom2 << 1) | _idCustom1 /* bits used for the EVEX.aaa context */
785783

786784
#if !defined(TARGET_ARMARCH)
787785
unsigned _idCustom4 : 1;
@@ -1594,30 +1592,36 @@ class emitter
15941592

15951593
bool idIsBound() const
15961594
{
1595+
assert(!IsAvx512OrPriorInstruction(_idIns));
15971596
return _idBound != 0;
15981597
}
15991598
void idSetIsBound()
16001599
{
1600+
assert(!IsAvx512OrPriorInstruction(_idIns));
16011601
_idBound = 1;
16021602
}
16031603

16041604
#ifndef TARGET_ARMARCH
16051605
bool idIsCallRegPtr() const
16061606
{
1607+
assert(!IsAvx512OrPriorInstruction(_idIns));
16071608
return _idCallRegPtr != 0;
16081609
}
16091610
void idSetIsCallRegPtr()
16101611
{
1612+
assert(!IsAvx512OrPriorInstruction(_idIns));
16111613
_idCallRegPtr = 1;
16121614
}
16131615
#endif // !TARGET_ARMARCH
16141616

16151617
bool idIsTlsGD() const
16161618
{
1619+
assert(!IsAvx512OrPriorInstruction(_idIns));
16171620
return _idTlsGD != 0;
16181621
}
16191622
void idSetTlsGD()
16201623
{
1624+
assert(!IsAvx512OrPriorInstruction(_idIns));
16211625
_idTlsGD = 1;
16221626
}
16231627

@@ -1626,10 +1630,12 @@ class emitter
16261630
// code, it is not necessary to generate GC info for a call so labeled.
16271631
bool idIsNoGC() const
16281632
{
1633+
assert(!IsAvx512OrPriorInstruction(_idIns));
16291634
return _idNoGC != 0;
16301635
}
16311636
void idSetIsNoGC(bool val)
16321637
{
1638+
assert(!IsAvx512OrPriorInstruction(_idIns));
16331639
_idNoGC = val;
16341640
}
16351641

@@ -1668,6 +1674,7 @@ class emitter
16681674

16691675
unsigned idGetEvexAaaContext() const
16701676
{
1677+
assert(IsAvx512OrPriorInstruction(_idIns));
16711678
return _idEvexAaaContext;
16721679
}
16731680

@@ -1683,6 +1690,7 @@ class emitter
16831690

16841691
bool idIsEvexZContextSet() const
16851692
{
1693+
assert(IsAvx512OrPriorInstruction(_idIns));
16861694
return _idEvexZContext != 0;
16871695
}
16881696

‎src/coreclr/jit/emitxarch.cpp‎

Lines changed: 4 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -49,23 +49,6 @@ bool emitter::IsKInstruction(instruction ins)
4949
return (flags & KInstruction) != 0;
5050
}
5151

52-
//------------------------------------------------------------------------
53-
// IsAvx512OrPriorInstruction: Is this an Avx512 or Avx or Sse or K (opmask) instruction.
54-
// Technically, K instructions would be considered under the VEX encoding umbrella, but due to
55-
// the instruction table encoding had to be pulled out with the rest of the `INST5` definitions.
56-
//
57-
// Arguments:
58-
// ins - The instruction to check.
59-
//
60-
// Returns:
61-
// `true` if it is a sse or avx or avx512 instruction.
62-
//
63-
bool emitter::IsAvx512OrPriorInstruction(instruction ins)
64-
{
65-
// TODO-XArch-AVX512: Fix check once AVX512 instructions are added.
66-
return ((ins >= INS_FIRST_SSE_INSTRUCTION) && (ins <= INS_LAST_AVX512_INSTRUCTION));
67-
}
68-
6952
bool emitter::IsAVXOnlyInstruction(instruction ins)
7053
{
7154
return (ins >= INS_FIRST_AVX_INSTRUCTION) && (ins <= INS_LAST_AVX_INSTRUCTION);
@@ -4189,9 +4172,8 @@ UNATIVE_OFFSET emitter::emitInsSizeAM(instrDesc* id, code_t code)
41894172
}
41904173

41914174
// If this is just "call reg", we're done.
4192-
if (id->idIsCallRegPtr())
4175+
if (((ins == INS_call) || (ins == INS_tail_i_jmp)) && id->idIsCallRegPtr())
41934176
{
4194-
assert(ins == INS_call || ins == INS_tail_i_jmp);
41954177
assert(dsp == 0);
41964178
return size;
41974179
}
@@ -11081,7 +11063,7 @@ void emitter::emitDispIns(
1108111063
case IF_AWR:
1108211064
case IF_ARW:
1108311065
{
11084-
if (id->idIsCallRegPtr())
11066+
if (((ins == INS_call) || (ins == INS_tail_i_jmp)) && id->idIsCallRegPtr())
1108511067
{
1108611068
printf("%s", emitRegName(id->idAddr()->iiaAddrMode.amBaseReg));
1108711069
}
@@ -12973,7 +12955,7 @@ BYTE* emitter::emitOutputAM(BYTE* dst, instrDesc* id, code_t code, CnsVal* addc)
1297312955
#else
1297412956
dst += emitOutputLong(dst, dsp);
1297512957
#endif
12976-
if (id->idIsTlsGD())
12958+
if ((ins == INS_call) && id->idIsTlsGD())
1297712959
{
1297812960
addlDelta = -4;
1297912961
emitRecordRelocationWithAddlDelta((void*)(dst - sizeof(INT32)), (void*)dsp, IMAGE_REL_TLSGD,
@@ -16703,7 +16685,7 @@ size_t emitter::emitOutputInstr(insGroup* ig, instrDesc* id, BYTE** dp)
1670316685
}
1670416686

1670516687
#ifdef DEBUG
16706-
if (ins == INS_call && !id->idIsTlsGD())
16688+
if ((ins == INS_call) && !id->idIsTlsGD())
1670716689
{
1670816690
emitRecordCallSite(emitCurCodeOffs(*dp), id->idDebugOnlyInfo()->idCallSig,
1670916691
(CORINFO_METHOD_HANDLE)id->idDebugOnlyInfo()->idMemCookie);

‎src/coreclr/jit/emitxarch.h‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -106,7 +106,6 @@ unsigned insSSval(unsigned scale);
106106

107107
static bool IsSSEInstruction(instruction ins);
108108
static bool IsSSEOrAVXInstruction(instruction ins);
109-
static bool IsAvx512OrPriorInstruction(instruction ins);
110109
static bool IsAVXOnlyInstruction(instruction ins);
111110
static bool IsAvx512OnlyInstruction(instruction ins);
112111
static bool IsFMAInstruction(instruction ins);

‎src/coreclr/jit/gentree.cpp‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -19773,7 +19773,10 @@ bool GenTree::isRMWHWIntrinsic(Compiler* comp)
1977319773
//
1977419774
bool GenTree::isEvexCompatibleHWIntrinsic() const
1977519775
{
19776-
return OperIsHWIntrinsic() && HWIntrinsicInfo::HasEvexSemantics(AsHWIntrinsic()->GetHWIntrinsicId());
19776+
// TODO-XARCH-AVX512 remove the ReturnsPerElementMask check once K registers have been properly
19777+
// implemented in the register allocator
19778+
return OperIsHWIntrinsic() && HWIntrinsicInfo::HasEvexSemantics(AsHWIntrinsic()->GetHWIntrinsicId()) &&
19779+
!HWIntrinsicInfo::ReturnsPerElementMask(AsHWIntrinsic()->GetHWIntrinsicId());
1977719780
}
1977819781

1977919782
//------------------------------------------------------------------------

‎src/coreclr/jit/instr.h‎

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -84,6 +84,26 @@ enum instruction : uint32_t
8484
INS_count = INS_none
8585
};
8686

87+
//------------------------------------------------------------------------
88+
// IsAvx512OrPriorInstruction: Is this an Avx512 or Avx or Sse or K (opmask) instruction.
89+
// Technically, K instructions would be considered under the VEX encoding umbrella, but due to
90+
// the instruction table encoding had to be pulled out with the rest of the `INST5` definitions.
91+
//
92+
// Arguments:
93+
// ins - The instruction to check.
94+
//
95+
// Returns:
96+
// `true` if it is a sse or avx or avx512 instruction.
97+
//
98+
inline bool IsAvx512OrPriorInstruction(instruction ins)
99+
{
100+
#if defined(TARGET_XARCH)
101+
return (ins >= INS_FIRST_SSE_INSTRUCTION) && (ins <= INS_LAST_AVX512_INSTRUCTION);
102+
#else
103+
return false;
104+
#endif // TARGET_XARCH
105+
}
106+
87107
/*****************************************************************************/
88108

89109
enum insUpdateModes

0 commit comments

Comments
 (0)