Skip to content

Commit

Permalink
Add some basic asserts to ensure _idCustom# isn't used incorrectly
Browse files Browse the repository at this point in the history
  • Loading branch information
tannergooding committed Jan 30, 2024
1 parent 1532212 commit 4a0eba9
Show file tree
Hide file tree
Showing 5 changed files with 39 additions and 27 deletions.
14 changes: 11 additions & 3 deletions src/coreclr/jit/emit.h
Original file line number Diff line number Diff line change
Expand Up @@ -779,9 +779,7 @@ class emitter
#define _idBound _idCustom1 /* jump target / frame offset bound */
#define _idTlsGD _idCustom2 /* Used to store information related to TLS GD access on linux */
#define _idNoGC _idCustom3 /* Some helpers don't get recorded in GC tables */
#define _idEvexAaaContext \
((_idCustom3 << 2) | (_idCustom2 << 1) | _idCustom1) /* bits used for the EVEX.aaa context \
*/
#define _idEvexAaaContext (_idCustom3 << 2) | (_idCustom2 << 1) | _idCustom1 /* bits used for the EVEX.aaa context */

#if !defined(TARGET_ARMARCH)
unsigned _idCustom4 : 1;
Expand Down Expand Up @@ -1594,30 +1592,36 @@ class emitter

bool idIsBound() const
{
assert(!IsAvx512OrPriorInstruction(_idIns));
return _idBound != 0;
}
void idSetIsBound()
{
assert(!IsAvx512OrPriorInstruction(_idIns));
_idBound = 1;
}

#ifndef TARGET_ARMARCH
bool idIsCallRegPtr() const
{
assert(!IsAvx512OrPriorInstruction(_idIns));
return _idCallRegPtr != 0;
}
void idSetIsCallRegPtr()
{
assert(!IsAvx512OrPriorInstruction(_idIns));
_idCallRegPtr = 1;
}
#endif // !TARGET_ARMARCH

bool idIsTlsGD() const
{
assert(!IsAvx512OrPriorInstruction(_idIns));
return _idTlsGD != 0;
}
void idSetTlsGD()
{
assert(!IsAvx512OrPriorInstruction(_idIns));
_idTlsGD = 1;
}

Expand All @@ -1626,10 +1630,12 @@ class emitter
// code, it is not necessary to generate GC info for a call so labeled.
bool idIsNoGC() const
{
assert(!IsAvx512OrPriorInstruction(_idIns));
return _idNoGC != 0;
}
void idSetIsNoGC(bool val)
{
assert(!IsAvx512OrPriorInstruction(_idIns));
_idNoGC = val;
}

Expand Down Expand Up @@ -1668,6 +1674,7 @@ class emitter

unsigned idGetEvexAaaContext() const
{
assert(IsAvx512OrPriorInstruction(_idIns));
return _idEvexAaaContext;
}

Expand All @@ -1683,6 +1690,7 @@ class emitter

bool idIsEvexZContextSet() const
{
assert(IsAvx512OrPriorInstruction(_idIns));
return _idEvexZContext != 0;
}

Expand Down
26 changes: 4 additions & 22 deletions src/coreclr/jit/emitxarch.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -49,23 +49,6 @@ bool emitter::IsKInstruction(instruction ins)
return (flags & KInstruction) != 0;
}

//------------------------------------------------------------------------
// IsAvx512OrPriorInstruction: Is this an Avx512 or Avx or Sse or K (opmask) instruction.
// Technically, K instructions would be considered under the VEX encoding umbrella, but due to
// the instruction table encoding had to be pulled out with the rest of the `INST5` definitions.
//
// Arguments:
// ins - The instruction to check.
//
// Returns:
// `true` if it is a sse or avx or avx512 instruction.
//
bool emitter::IsAvx512OrPriorInstruction(instruction ins)
{
// TODO-XArch-AVX512: Fix check once AVX512 instructions are added.
return ((ins >= INS_FIRST_SSE_INSTRUCTION) && (ins <= INS_LAST_AVX512_INSTRUCTION));
}

bool emitter::IsAVXOnlyInstruction(instruction ins)
{
return (ins >= INS_FIRST_AVX_INSTRUCTION) && (ins <= INS_LAST_AVX_INSTRUCTION);
Expand Down Expand Up @@ -4189,9 +4172,8 @@ UNATIVE_OFFSET emitter::emitInsSizeAM(instrDesc* id, code_t code)
}

// If this is just "call reg", we're done.
if (id->idIsCallRegPtr())
if (((ins == INS_call) || (ins == INS_tail_i_jmp)) && id->idIsCallRegPtr())
{
assert(ins == INS_call || ins == INS_tail_i_jmp);
assert(dsp == 0);
return size;
}
Expand Down Expand Up @@ -11081,7 +11063,7 @@ void emitter::emitDispIns(
case IF_AWR:
case IF_ARW:
{
if (id->idIsCallRegPtr())
if (((ins == INS_call) || (ins == INS_tail_i_jmp)) && id->idIsCallRegPtr())
{
printf("%s", emitRegName(id->idAddr()->iiaAddrMode.amBaseReg));
}
Expand Down Expand Up @@ -12973,7 +12955,7 @@ BYTE* emitter::emitOutputAM(BYTE* dst, instrDesc* id, code_t code, CnsVal* addc)
#else
dst += emitOutputLong(dst, dsp);
#endif
if (id->idIsTlsGD())
if ((ins == INS_call) && id->idIsTlsGD())
{
addlDelta = -4;
emitRecordRelocationWithAddlDelta((void*)(dst - sizeof(INT32)), (void*)dsp, IMAGE_REL_TLSGD,
Expand Down Expand Up @@ -16703,7 +16685,7 @@ size_t emitter::emitOutputInstr(insGroup* ig, instrDesc* id, BYTE** dp)
}

#ifdef DEBUG
if (ins == INS_call && !id->idIsTlsGD())
if ((ins == INS_call) && !id->idIsTlsGD())
{
emitRecordCallSite(emitCurCodeOffs(*dp), id->idDebugOnlyInfo()->idCallSig,
(CORINFO_METHOD_HANDLE)id->idDebugOnlyInfo()->idMemCookie);
Expand Down
1 change: 0 additions & 1 deletion src/coreclr/jit/emitxarch.h
Original file line number Diff line number Diff line change
Expand Up @@ -106,7 +106,6 @@ unsigned insSSval(unsigned scale);

static bool IsSSEInstruction(instruction ins);
static bool IsSSEOrAVXInstruction(instruction ins);
static bool IsAvx512OrPriorInstruction(instruction ins);
static bool IsAVXOnlyInstruction(instruction ins);
static bool IsAvx512OnlyInstruction(instruction ins);
static bool IsFMAInstruction(instruction ins);
Expand Down
5 changes: 4 additions & 1 deletion src/coreclr/jit/gentree.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -19773,7 +19773,10 @@ bool GenTree::isRMWHWIntrinsic(Compiler* comp)
//
bool GenTree::isEvexCompatibleHWIntrinsic() const
{
return OperIsHWIntrinsic() && HWIntrinsicInfo::HasEvexSemantics(AsHWIntrinsic()->GetHWIntrinsicId());
// TODO-XARCH-AVX512 remove the ReturnsPerElementMask check once K registers have been properly
// implemented in the register allocator
return OperIsHWIntrinsic() && HWIntrinsicInfo::HasEvexSemantics(AsHWIntrinsic()->GetHWIntrinsicId()) &&
!HWIntrinsicInfo::ReturnsPerElementMask(AsHWIntrinsic()->GetHWIntrinsicId());
}

//------------------------------------------------------------------------
Expand Down
20 changes: 20 additions & 0 deletions src/coreclr/jit/instr.h
Original file line number Diff line number Diff line change
Expand Up @@ -84,6 +84,26 @@ enum instruction : uint32_t
INS_count = INS_none
};

//------------------------------------------------------------------------
// IsAvx512OrPriorInstruction: Is this an Avx512 or Avx or Sse or K (opmask) instruction.
// Technically, K instructions would be considered under the VEX encoding umbrella, but due to
// the instruction table encoding had to be pulled out with the rest of the `INST5` definitions.
//
// Arguments:
// ins - The instruction to check.
//
// Returns:
// `true` if it is a sse or avx or avx512 instruction.
//
inline bool IsAvx512OrPriorInstruction(instruction ins)
{
#if defined(TARGET_XARCH)
return (ins >= INS_FIRST_SSE_INSTRUCTION) && (ins <= INS_LAST_AVX512_INSTRUCTION);
#else
return false;
#endif // TARGET_XARCH
}

/*****************************************************************************/

enum insUpdateModes
Expand Down

0 comments on commit 4a0eba9

Please sign in to comment.