Skip to content

Commit

Permalink
[X86][tablgen] Refine the class RecognizableInstr. NFCI
Browse files Browse the repository at this point in the history
1. Add comments to explain why we set `isAsmParserOnly` for XACQUIRE and XRELEASE
2. Check `X86Inst` in the constructor of `RecognizableInstrBase` so that
   we can avoid the case where one of it's field is not initialized but
   accessed by user. (e.g. in X86EVEX2VEXTablesEmitter.cpp)
3. Move `Rec` from `RecognizableInstrBase` to `RecognizableInstr` to reduce
   size of `RecognizableInstrBase`
4. Remove out-of-date comments for shouldBeEmitted() (filter() was removed)
5. Add a basic field `IsAsmParserOnly` and remove the field
   `ShouldBeEmitted` b/c we can deduce it w/ little overhead
  • Loading branch information
KanRobert committed Mar 26, 2022
1 parent bfa2f25 commit 271e8d2
Show file tree
Hide file tree
Showing 6 changed files with 36 additions and 44 deletions.
2 changes: 2 additions & 0 deletions llvm/lib/Target/X86/X86InstrTSX.td
Expand Up @@ -51,6 +51,8 @@ def XABORT : Ii8<0xc6, MRM_F8, (outs), (ins i8imm:$imm),
// HLE prefixes
let SchedRW = [WriteSystem] in {

// XACQUIRE and XRELEASE reuse REPNE and REP respectively.
// For now, just prefer the REP versions.
let isAsmParserOnly = 1 in {
def XACQUIRE_PREFIX : I<0xF2, PrefixByte, (outs), (ins), "xacquire", []>;
def XRELEASE_PREFIX : I<0xF3, PrefixByte, (outs), (ins), "xrelease", []>;
Expand Down
6 changes: 3 additions & 3 deletions llvm/utils/TableGen/X86EVEX2VEXTablesEmitter.cpp
Expand Up @@ -116,7 +116,7 @@ class IsMatch {
bool EVEX_W = EVEXRI.HasVEX_W;
bool VEX_WIG = VEXRI.IgnoresVEX_W;
bool EVEX_WIG = EVEXRI.IgnoresVEX_W;
bool EVEX_W1_VEX_W0 = EVEXRI.Rec->getValueAsBit("EVEX_W1_VEX_W0");
bool EVEX_W1_VEX_W0 = EVEXInst->TheDef->getValueAsBit("EVEX_W1_VEX_W0");

if (VEXRI.IsCodeGenOnly != EVEXRI.IsCodeGenOnly ||
// VEX/EVEX fields
Expand Down Expand Up @@ -205,11 +205,11 @@ void X86EVEX2VEXTablesEmitter::run(raw_ostream &OS) {
Target.getInstructionsByEnumValue();

for (const CodeGenInstruction *Inst : NumberedInstructions) {
X86Disassembler::RecognizableInstrBase RI(*Inst);
const Record *Def = RI.Rec;
const Record *Def = Inst->TheDef;
// Filter non-X86 instructions.
if (!Def->isSubClassOf("X86Inst"))
continue;
X86Disassembler::RecognizableInstrBase RI(*Inst);

// Add VEX encoded instructions to one of VEXInsts vectors according to
// it's opcode.
Expand Down
4 changes: 2 additions & 2 deletions llvm/utils/TableGen/X86FoldTablesEmitter.cpp
Expand Up @@ -311,8 +311,8 @@ class IsMatch {
bool operator()(const CodeGenInstruction *RegInst) {
X86Disassembler::RecognizableInstrBase RegRI(*RegInst);
X86Disassembler::RecognizableInstrBase MemRI(*MemInst);
const Record *RegRec = RegRI.Rec;
const Record *MemRec = MemRI.Rec;
const Record *RegRec = RegInst->TheDef;
const Record *MemRec = MemInst->TheDef;

// EVEX_B means different things for memory and register forms.
if (RegRI.HasEVEX_B != 0 || MemRI.HasEVEX_B != 0)
Expand Down
7 changes: 5 additions & 2 deletions llvm/utils/TableGen/X86MnemonicTables.cpp
Expand Up @@ -43,9 +43,12 @@ void X86MnemonicTablesEmitter::run(raw_ostream &OS) {
ArrayRef<const CodeGenInstruction *> NumberedInstructions =
Target.getInstructionsByEnumValue();
for (const CodeGenInstruction *I : NumberedInstructions) {
const Record *Def = I->TheDef;
// Filter non-X86 instructions.
if (!Def->isSubClassOf("X86Inst"))
continue;
X86Disassembler::RecognizableInstrBase RI(*I);
const Record *Def = RI.Rec;
if (!RI.ShouldBeEmitted)
if (!RI.shouldBeEmitted())
continue;
if ( // Non-parsable instruction defs contain prefix as part of AsmString
Def->getValueAsString("AsmVariantName") == "NonParsable" ||
Expand Down
36 changes: 18 additions & 18 deletions llvm/utils/TableGen/X86RecognizableInstr.cpp
Expand Up @@ -75,13 +75,9 @@ static uint8_t byteFromRec(const Record* rec, StringRef name) {
return byteFromBitsInit(*bits);
}

RecognizableInstrBase::RecognizableInstrBase(const CodeGenInstruction &insn)
: Rec(insn.TheDef),
ShouldBeEmitted(Rec->isSubClassOf("X86Inst") &&
!Rec->getValueAsBit("isAsmParserOnly")) {
if (!ShouldBeEmitted)
return;

RecognizableInstrBase::RecognizableInstrBase(const CodeGenInstruction &insn) {
const Record *Rec = insn.TheDef;
assert(Rec->isSubClassOf("X86Inst") && "Not a X86 Instruction");
OpPrefix = byteFromRec(Rec, "OpPrefixBits");
OpMap = byteFromRec(Rec, "OpMapBits");
Opcode = byteFromRec(Rec, "Opcode");
Expand All @@ -99,23 +95,26 @@ RecognizableInstrBase::RecognizableInstrBase(const CodeGenInstruction &insn)
HasEVEX_KZ = Rec->getValueAsBit("hasEVEX_Z");
HasEVEX_B = Rec->getValueAsBit("hasEVEX_B");
IsCodeGenOnly = Rec->getValueAsBit("isCodeGenOnly");
IsAsmParserOnly = Rec->getValueAsBit("isAsmParserOnly");
ForceDisassemble = Rec->getValueAsBit("ForceDisassemble");
CD8_Scale = byteFromRec(Rec, "CD8_Scale");
HasVEX_LPrefix = Rec->getValueAsBit("hasVEX_L");

EncodeRC = HasEVEX_B &&
(Form == X86Local::MRMDestReg || Form == X86Local::MRMSrcReg);
}

if (Form == X86Local::Pseudo || (IsCodeGenOnly && !ForceDisassemble))
ShouldBeEmitted = false;
bool RecognizableInstrBase::shouldBeEmitted() const {
return Form != X86Local::Pseudo && (!IsCodeGenOnly || ForceDisassemble) &&
!IsAsmParserOnly;
}

RecognizableInstr::RecognizableInstr(DisassemblerTables &tables,
const CodeGenInstruction &insn,
InstrUID uid)
: RecognizableInstrBase(insn), Name(Rec->getName().str()), Is32Bit(false),
Is64Bit(false), Operands(&insn.Operands.OperandList), UID(uid),
Spec(&tables.specForUID(uid)) {
: RecognizableInstrBase(insn), Rec(insn.TheDef), Name(Rec->getName().str()),
Is32Bit(false), Is64Bit(false), Operands(&insn.Operands.OperandList),
UID(uid), Spec(&tables.specForUID(uid)) {
// Check for 64-bit inst which does not require REX
// FIXME: Is there some better way to check for In64BitMode?
std::vector<Record *> Predicates = Rec->getValueAsListOfDefs("Predicates");
Expand All @@ -134,14 +133,15 @@ RecognizableInstr::RecognizableInstr(DisassemblerTables &tables,

void RecognizableInstr::processInstr(DisassemblerTables &tables,
const CodeGenInstruction &insn,
InstrUID uid)
{
InstrUID uid) {
if (!insn.TheDef->isSubClassOf("X86Inst"))
return;
RecognizableInstr recogInstr(tables, insn, uid);

if (recogInstr.shouldBeEmitted()) {
recogInstr.emitInstructionSpecifier();
recogInstr.emitDecodePath(tables);
}
if (!recogInstr.shouldBeEmitted())
return;
recogInstr.emitInstructionSpecifier();
recogInstr.emitDecodePath(tables);
}

#define EVEX_KB(n) (HasEVEX_KZ && HasEVEX_B ? n##_KZ_B : \
Expand Down
25 changes: 6 additions & 19 deletions llvm/utils/TableGen/X86RecognizableInstr.h
Expand Up @@ -160,8 +160,6 @@ class DisassemblerTables;

/// Extract common fields of a single X86 instruction from a CodeGenInstruction
struct RecognizableInstrBase {
/// The record from the .td files corresponding to this instruction
const Record* Rec;
/// The OpPrefix field from the record
uint8_t OpPrefix;
/// The OpMap field from the record
Expand Down Expand Up @@ -201,16 +199,16 @@ struct RecognizableInstrBase {
bool EncodeRC;
/// The isCodeGenOnly field from the record
bool IsCodeGenOnly;
/// The isAsmParserOnly field from the record
bool IsAsmParserOnly;
/// The ForceDisassemble field from the record
bool ForceDisassemble;
// The CD8_Scale field from the record
uint8_t CD8_Scale;
/// Indicates whether the instruction should be emitted into the decode
/// tables; regardless, it will be emitted into the instruction info table
bool ShouldBeEmitted;

/// \param insn The CodeGenInstruction to extract information from.
RecognizableInstrBase(const CodeGenInstruction &insn);
/// \returns true if this instruction should be emitted
bool shouldBeEmitted() const;
};

/// RecognizableInstr - Encapsulates all information required to decode a single
Expand All @@ -219,6 +217,8 @@ struct RecognizableInstrBase {
/// instruction into DisassemblerTables.
class RecognizableInstr : public RecognizableInstrBase {
private:
/// The record from the .td files corresponding to this instruction
const Record* Rec;
/// The instruction name as listed in the tables
std::string Name;
// Whether the instruction has the predicate "In32BitMode"
Expand Down Expand Up @@ -318,19 +318,6 @@ class RecognizableInstr : public RecognizableInstrBase {
(const std::string&,
uint8_t OpSize));

/// shouldBeEmitted - Returns the shouldBeEmitted field. Although filter()
/// filters out many instructions, at various points in decoding we
/// determine that the instruction should not actually be decodable. In
/// particular, MMX MOV instructions aren't emitted, but they're only
/// identified during operand parsing.
///
/// @return - true if at this point we believe the instruction should be
/// emitted; false if not. This will return false if filter() returns false
/// once emitInstructionSpecifier() has been called.
bool shouldBeEmitted() const {
return ShouldBeEmitted;
}

/// emitInstructionSpecifier - Loads the instruction specifier for the current
/// instruction into a DisassemblerTables.
///
Expand Down

0 comments on commit 271e8d2

Please sign in to comment.