Skip to content

Commit

Permalink
[TableGen][GlobalISel] Specialize more MatchTable Opcodes (#89736)
Browse files Browse the repository at this point in the history 
The vast majority of the following (very common) opcodes were always
called with identical arguments:

- `GIM_CheckType` for the root
- `GIM_CheckRegBankForClass` for the root
- `GIR_Copy` between the old and new root
- `GIR_ConstrainSelectedInstOperands` on the new root
- `GIR_BuildMI` to create the new root

I added overloaded version of each opcode specialized for the root
instructions. It always saves between 1 and 2 bytes per instance
depending on the number of arguments specialized into the opcode. Some
of these opcodes had between 5 and 15k occurences in the AArch64
GlobalISel Match Table.

Additionally, the following opcodes are almost always used in the same
sequence:

- `GIR_EraseFromParent 0` + `GIR_Done` 
- `GIR_EraseRootFromParent_Done` has been created to do both. Saves 2
bytes per occurence.
- `GIR_IsSafeToFold` was *always* called for each InsnID except 0.
- Changed the opcode to take the number of instructions to check after
`MI[0]`

The savings from these are pretty neat. For `AArch64GenGlobalISel.inc`:
- `AArch64InstructionSelector.cpp.o` goes down from 772kb to 704kb (-10%
code size)
- Self-reported MatchTable size goes from 420380 bytes to 352426 bytes
(~ -17%)

A smaller match table means a faster match table because we spend less
time iterating and decoding.
I don't have a solid measurement methodology for GlobalISel performance
so I don't have precise numbers but I saw a few % of improvements in a
simple testcase.
  • Loading branch information
@Pierre-vh
Pierre-vh committed Apr 24, 2024
1 parent 7da6342 commit 9375962
Show file tree
Hide file tree
Showing 43 changed files with 1,073 additions and 1,042 deletions.
20 changes: 17 additions & 3 deletions llvm/include/llvm/CodeGen/GlobalISel/GIMatchTableExecutor.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -217,6 +217,8 @@ enum {
/// - OpIdx(ULEB128) - Operand index
/// - Ty(1) - Expected type
GIM_CheckType,
/// GIM_CheckType but InsnID is omitted and defaults to zero.
GIM_RootCheckType,

/// Check the type of a pointer to any address space.
/// - InsnID(ULEB128) - Instruction ID
Expand All @@ -229,6 +231,8 @@ enum {
/// - OpIdx(ULEB128) - Operand index
/// - RC(2) - Expected register bank (specified as a register class)
GIM_CheckRegBankForClass,
/// GIM_CheckRegBankForClass but InsnID is omitted and defaults to zero.
GIM_RootCheckRegBankForClass,

/// Check the operand matches a complex predicate
/// - InsnID(ULEB128) - Instruction ID
Expand Down Expand Up @@ -278,9 +282,9 @@ enum {
/// - OpIdx(ULEB128) - Operand index
GIM_CheckIsImm,

/// Check if the specified operand is safe to fold into the current
/// instruction.
/// - InsnID(ULEB128) - Instruction ID
/// Checks if the matched instructions numbered [1, 1+N) can
/// be folded into the root (inst 0).
/// - Num(1)
GIM_CheckIsSafeToFold,

/// Check the specified operands are identical.
Expand Down Expand Up @@ -338,6 +342,8 @@ enum {
/// - InsnID(ULEB128) - Instruction ID to define
/// - Opcode(2) - The new opcode to use
GIR_BuildMI,
/// GIR_BuildMI but InsnID is omitted and defaults to zero.
GIR_BuildRootMI,

/// Builds a constant and stores its result in a TempReg.
/// - TempRegID(ULEB128) - Temp Register to define.
Expand All @@ -349,6 +355,8 @@ enum {
/// - OldInsnID(ULEB128) - Instruction ID to copy from
/// - OpIdx(ULEB128) - The operand to copy
GIR_Copy,
/// GIR_Copy but with both New/OldInsnIDs omitted and defaulting to zero.
GIR_RootToRootCopy,

/// Copy an operand to the specified instruction or add a zero register if the
/// operand is a zero immediate.
Expand Down Expand Up @@ -506,6 +514,9 @@ enum {
/// description.
/// - InsnID(ULEB128) - Instruction ID to modify
GIR_ConstrainSelectedInstOperands,
/// GIR_ConstrainSelectedInstOperands but InsnID is omitted and defaults to
/// zero.
GIR_RootConstrainSelectedInstOperands,

/// Merge all memory operands into instruction.
/// - InsnID(ULEB128) - Instruction ID to modify
Expand All @@ -518,6 +529,9 @@ enum {
/// - InsnID(ULEB128) - Instruction ID to erase
GIR_EraseFromParent,

/// Combines both a GIR_EraseFromParent 0 + GIR_Done
GIR_EraseRootFromParent_Done,

/// Create a new temporary register that's not constrained.
/// - TempRegID(ULEB128) - The temporary register ID to initialize.
/// - Ty(1) - Expected type
Expand Down
68 changes: 45 additions & 23 deletions llvm/include/llvm/CodeGen/GlobalISel/GIMatchTableExecutorImpl.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -131,6 +131,16 @@ bool GIMatchTableExecutor::executeMatchTable(
return V;
};

const auto eraseImpl = [&](MachineInstr *MI) {
// If we're erasing the insertion point, ensure we don't leave a dangling
// pointer in the builder.
if (Builder.getInsertPt() == MI)
Builder.setInsertPt(*MI->getParent(), ++MI->getIterator());
if (Observer)
Observer->erasingInstr(*MI);
MI->eraseFromParent();
};

while (true) {
assert(CurrentIdx != ~0u && "Invalid MatchTable index");
uint8_t MatcherOpcode = MatchTable[CurrentIdx++];
Expand Down Expand Up @@ -661,8 +671,9 @@ bool GIMatchTableExecutor::executeMatchTable(

break;
}
case GIM_RootCheckType:
case GIM_CheckType: {
uint64_t InsnID = readULEB();
uint64_t InsnID = (MatcherOpcode == GIM_RootCheckType) ? 0 : readULEB();
uint64_t OpIdx = readULEB();
int TypeID = readS8();
DEBUG_WITH_TYPE(TgtExecutor::getName(),
Expand Down Expand Up @@ -741,8 +752,11 @@ bool GIMatchTableExecutor::executeMatchTable(
State.RecordedTypes[TypeIdx] = MRI.getType(Op.getReg());
break;
}

case GIM_RootCheckRegBankForClass:
case GIM_CheckRegBankForClass: {
uint64_t InsnID = readULEB();
uint64_t InsnID =
(MatcherOpcode == GIM_RootCheckRegBankForClass) ? 0 : readULEB();
uint64_t OpIdx = readULEB();
uint16_t RCEnum = readU16();
DEBUG_WITH_TYPE(TgtExecutor::getName(),
Expand Down Expand Up @@ -898,14 +912,16 @@ bool GIMatchTableExecutor::executeMatchTable(
break;
}
case GIM_CheckIsSafeToFold: {
uint64_t InsnID = readULEB();
uint64_t NumInsn = MatchTable[CurrentIdx++];
DEBUG_WITH_TYPE(TgtExecutor::getName(),
dbgs() << CurrentIdx << ": GIM_CheckIsSafeToFold(MIs["
<< InsnID << "])\n");
assert(State.MIs[InsnID] != nullptr && "Used insn before defined");
if (!isObviouslySafeToFold(*State.MIs[InsnID], *State.MIs[0])) {
if (handleReject() == RejectAndGiveUp)
return false;
dbgs() << CurrentIdx << ": GIM_CheckIsSafeToFold(N = "
<< NumInsn << ")\n");
MachineInstr &Root = *State.MIs[0];
for (unsigned K = 1, E = NumInsn + 1; K < E; ++K) {
if (!isObviouslySafeToFold(*State.MIs[K], Root)) {
if (handleReject() == RejectAndGiveUp)
return false;
}
}
break;
}
Expand Down Expand Up @@ -1011,8 +1027,9 @@ bool GIMatchTableExecutor::executeMatchTable(
break;
}

case GIR_BuildRootMI:
case GIR_BuildMI: {
uint64_t NewInsnID = readULEB();
uint64_t NewInsnID = (MatcherOpcode == GIR_BuildRootMI) ? 0 : readULEB();
uint16_t Opcode = readU16();
if (NewInsnID >= OutMIs.size())
OutMIs.resize(NewInsnID + 1);
Expand All @@ -1034,9 +1051,12 @@ bool GIMatchTableExecutor::executeMatchTable(
break;
}

case GIR_RootToRootCopy:
case GIR_Copy: {
uint64_t NewInsnID = readULEB();
uint64_t OldInsnID = readULEB();
uint64_t NewInsnID =
(MatcherOpcode == GIR_RootToRootCopy) ? 0 : readULEB();
uint64_t OldInsnID =
(MatcherOpcode == GIR_RootToRootCopy) ? 0 : readULEB();
uint64_t OpIdx = readULEB();
assert(OutMIs[NewInsnID] && "Attempted to add to undefined instruction");
OutMIs[NewInsnID].add(State.MIs[OldInsnID]->getOperand(OpIdx));
Expand Down Expand Up @@ -1361,8 +1381,11 @@ bool GIMatchTableExecutor::executeMatchTable(
break;
}

case GIR_RootConstrainSelectedInstOperands:
case GIR_ConstrainSelectedInstOperands: {
uint64_t InsnID = readULEB();
uint64_t InsnID = (MatcherOpcode == GIR_RootConstrainSelectedInstOperands)
? 0
: readULEB();
assert(OutMIs[InsnID] && "Attempted to add to undefined instruction");
constrainSelectedInstRegOperands(*OutMIs[InsnID].getInstr(), TII, TRI,
RBI);
Expand All @@ -1372,7 +1395,6 @@ bool GIMatchTableExecutor::executeMatchTable(
<< InsnID << "])\n");
break;
}

case GIR_MergeMemOperands: {
uint64_t InsnID = readULEB();
uint64_t NumInsn = MatchTable[CurrentIdx++];
Expand All @@ -1391,24 +1413,24 @@ bool GIMatchTableExecutor::executeMatchTable(
DEBUG_WITH_TYPE(TgtExecutor::getName(), dbgs() << ")\n");
break;
}

case GIR_EraseFromParent: {
uint64_t InsnID = readULEB();
MachineInstr *MI = State.MIs[InsnID];
assert(MI && "Attempted to erase an undefined instruction");
DEBUG_WITH_TYPE(TgtExecutor::getName(),
dbgs() << CurrentIdx << ": GIR_EraseFromParent(MIs["
<< InsnID << "])\n");
// If we're erasing the insertion point, ensure we don't leave a dangling
// pointer in the builder.
if (Builder.getInsertPt() == MI)
Builder.setInsertPt(*MI->getParent(), ++MI->getIterator());
if (Observer)
Observer->erasingInstr(*MI);
MI->eraseFromParent();
eraseImpl(MI);
break;
}

case GIR_EraseRootFromParent_Done: {
DEBUG_WITH_TYPE(TgtExecutor::getName(),
dbgs()
<< CurrentIdx << ": GIR_EraseRootFromParent_Done\n");
eraseImpl(State.MIs[0]);
propagateFlags();
return true;
}
case GIR_MakeTempReg: {
uint64_t TempRegID = readULEB();
int TypeID = readS8();
Expand Down
32 changes: 16 additions & 16 deletions llvm/test/TableGen/ContextlessPredicates.td
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,50 +22,50 @@ def : Pat<(test_atomic_op_frag GPR32:$ptr, GPR32:$val) ,

// CHECK_NOPT-LABEL: const uint8_t *MyTargetInstructionSelector::getMatchTable() const {
// CHECK_NOPT-NEXT: constexpr static uint8_t MatchTable0[] = {
// CHECK_NOPT-NEXT: GIM_Try, /*On fail goto*//*Label 0*/ GIMT_Encode4(58), // Rule ID 0 //
// CHECK_NOPT-NEXT: GIM_Try, /*On fail goto*//*Label 0*/ GIMT_Encode4(52), // Rule ID 0 //
// CHECK_NOPT-NEXT: GIM_CheckNumOperands, /*MI*/0, /*Expected*/3,
// CHECK_NOPT-NEXT: GIM_CheckOpcode, /*MI*/0, GIMT_Encode2(TargetOpcode::G_ATOMICRMW_XCHG),
// CHECK_NOPT-NEXT: GIM_CheckMemorySizeEqualTo, /*MI*/0, /*MMO*/0, /*Size*/GIMT_Encode4(4),
// CHECK_NOPT-NEXT: // MIs[0] DstI[dst]
// CHECK_NOPT-NEXT: GIM_CheckType, /*MI*/0, /*Op*/0, /*Type*/GILLT_s32,
// CHECK_NOPT-NEXT: GIM_CheckRegBankForClass, /*MI*/0, /*Op*/0, /*RC*/GIMT_Encode2(MyTarget::GPR32RegClassID),
// CHECK_NOPT-NEXT: GIM_RootCheckType, /*Op*/0, /*Type*/GILLT_s32,
// CHECK_NOPT-NEXT: GIM_RootCheckRegBankForClass, /*Op*/0, /*RC*/GIMT_Encode2(MyTarget::GPR32RegClassID),
// CHECK_NOPT-NEXT: // MIs[0] ptr
// CHECK_NOPT-NEXT: GIM_CheckPointerToAny, /*MI*/0, /*Op*/1, /*SizeInBits*/32,
// CHECK_NOPT-NEXT: GIM_CheckRegBankForClass, /*MI*/0, /*Op*/1, /*RC*/GIMT_Encode2(MyTarget::GPR32RegClassID),
// CHECK_NOPT-NEXT: GIM_RootCheckRegBankForClass, /*Op*/1, /*RC*/GIMT_Encode2(MyTarget::GPR32RegClassID),
// CHECK_NOPT-NEXT: // MIs[0] val
// CHECK_NOPT-NEXT: GIM_CheckType, /*MI*/0, /*Op*/2, /*Type*/GILLT_s32,
// CHECK_NOPT-NEXT: GIM_CheckRegBankForClass, /*MI*/0, /*Op*/2, /*RC*/GIMT_Encode2(MyTarget::GPR32RegClassID),
// CHECK_NOPT-NEXT: GIM_RootCheckType, /*Op*/2, /*Type*/GILLT_s32,
// CHECK_NOPT-NEXT: GIM_RootCheckRegBankForClass, /*Op*/2, /*RC*/GIMT_Encode2(MyTarget::GPR32RegClassID),
// CHECK_NOPT-NEXT: GIM_CheckCxxInsnPredicate, /*MI*/0, /*FnId*/GIMT_Encode2(GICXXPred_MI_Predicate_test_atomic_op_frag),
// CHECK_NOPT-NEXT: // (atomic_swap:{ *:[i32] } GPR32:{ *:[i32] }:$ptr, GPR32:{ *:[i32] }:$val)<<P:Predicate_test_atomic_op_frag>> => (INSN:{ *:[i32] } GPR32:{ *:[i32] }:$ptr, GPR32:{ *:[i32] }:$val)
// CHECK_NOPT-NEXT: GIR_MutateOpcode, /*InsnID*/0, /*RecycleInsnID*/0, /*Opcode*/GIMT_Encode2(MyTarget::INSN),
// CHECK_NOPT-NEXT: GIR_ConstrainSelectedInstOperands, /*InsnID*/0,
// CHECK_NOPT-NEXT: GIR_RootConstrainSelectedInstOperands,
// CHECK_NOPT-NEXT: // GIR_Coverage, 0,
// CHECK_NOPT-NEXT: GIR_Done,
// CHECK_NOPT-NEXT: // Label 0: @58
// CHECK_NOPT-NEXT: // Label 0: @52
// CHECK_NOPT-NEXT: GIM_Reject,
// CHECK_NOPT-NEXT: };
// CHECK_NOPT-NEXT: return MatchTable0;
// CHECK_NOPT-NEXT: }

// CHECK_OPT-LABEL: const uint8_t *MyTargetInstructionSelector::getMatchTable() const {
// CHECK_OPT-NEXT: constexpr static uint8_t MatchTable0[] = {
// CHECK_OPT-NEXT: GIM_Try, /*On fail goto*//*Label 0*/ GIMT_Encode4(55), // Rule ID 0 //
// CHECK_OPT-NEXT: GIM_Try, /*On fail goto*//*Label 0*/ GIMT_Encode4(49), // Rule ID 0 //
// CHECK_OPT-NEXT: GIM_CheckOpcode, /*MI*/0, GIMT_Encode2(TargetOpcode::G_ATOMICRMW_XCHG),
// CHECK_OPT-NEXT: GIM_CheckType, /*MI*/0, /*Op*/0, /*Type*/GILLT_s32,
// CHECK_OPT-NEXT: GIM_CheckType, /*MI*/0, /*Op*/2, /*Type*/GILLT_s32,
// CHECK_OPT-NEXT: GIM_RootCheckType, /*Op*/0, /*Type*/GILLT_s32,
// CHECK_OPT-NEXT: GIM_RootCheckType, /*Op*/2, /*Type*/GILLT_s32,
// CHECK_OPT-NEXT: GIM_CheckMemorySizeEqualTo, /*MI*/0, /*MMO*/0, /*Size*/GIMT_Encode4(4),
// CHECK_OPT-NEXT: GIM_CheckRegBankForClass, /*MI*/0, /*Op*/0, /*RC*/GIMT_Encode2(MyTarget::GPR32RegClassID),
// CHECK_OPT-NEXT: GIM_RootCheckRegBankForClass, /*Op*/0, /*RC*/GIMT_Encode2(MyTarget::GPR32RegClassID),
// CHECK_OPT-NEXT: // MIs[0] ptr
// CHECK_OPT-NEXT: GIM_CheckPointerToAny, /*MI*/0, /*Op*/1, /*SizeInBits*/32,
// CHECK_OPT-NEXT: GIM_CheckRegBankForClass, /*MI*/0, /*Op*/1, /*RC*/GIMT_Encode2(MyTarget::GPR32RegClassID),
// CHECK_OPT-NEXT: GIM_CheckRegBankForClass, /*MI*/0, /*Op*/2, /*RC*/GIMT_Encode2(MyTarget::GPR32RegClassID),
// CHECK_OPT-NEXT: GIM_RootCheckRegBankForClass, /*Op*/1, /*RC*/GIMT_Encode2(MyTarget::GPR32RegClassID),
// CHECK_OPT-NEXT: GIM_RootCheckRegBankForClass, /*Op*/2, /*RC*/GIMT_Encode2(MyTarget::GPR32RegClassID),
// CHECK_OPT-NEXT: GIM_CheckCxxInsnPredicate, /*MI*/0, /*FnId*/GIMT_Encode2(GICXXPred_MI_Predicate_test_atomic_op_frag),
// CHECK_OPT-NEXT: // (atomic_swap:{ *:[i32] } GPR32:{ *:[i32] }:$ptr, GPR32:{ *:[i32] }:$val)<<P:Predicate_test_atomic_op_frag>> => (INSN:{ *:[i32] } GPR32:{ *:[i32] }:$ptr, GPR32:{ *:[i32] }:$val)
// CHECK_OPT-NEXT: GIR_MutateOpcode, /*InsnID*/0, /*RecycleInsnID*/0, /*Opcode*/GIMT_Encode2(MyTarget::INSN),
// CHECK_OPT-NEXT: GIR_ConstrainSelectedInstOperands, /*InsnID*/0,
// CHECK_OPT-NEXT: GIR_RootConstrainSelectedInstOperands,
// CHECK_OPT-NEXT: // GIR_Coverage, 0,
// CHECK_OPT-NEXT: GIR_Done,
// CHECK_OPT-NEXT: // Label 0: @55
// CHECK_OPT-NEXT: // Label 0: @49
// CHECK_OPT-NEXT: GIM_Reject,
// CHECK_OPT-NEXT: };
// CHECK_OPT-NEXT: return MatchTable0;
Expand Down
Loading

0 comments on commit 9375962

Please sign in to comment.