Skip to content

Commit

Permalink
[GlobalISel] Improving InstructionSelect's performance by reducing Ma…
Browse files Browse the repository at this point in the history 
…tchTable, mostly NFC, perf patch 1

This patch starts a series of patches that decrease time spent by
GlobalISel in its InstructionSelect pass by roughly 60% for -O0 builds
for large inputs as measured on sqlite3-amalgamation
(http://sqlite.org/download.html) targeting AArch64.

The performance improvements are achieved solely by reducing the
number of matching GIM_* opcodes executed by the MatchTable's
interpreter during the selection by approx. a factor of 30, which also
brings contribution of this particular part of the selection process
to the overall runtime of InstructionSelect pass down from approx.
60-70% to 5-7%, thus making further improvements in this particular
direction not very profitable.

The improvements described above are expected for any target that
doesn't have many complex patterns. The targets that do should
strictly benefit from the changes, but by how much exactly is hard to
estimate beforehand. It's also likely that such target WILL benefit
from further improvements to MatchTable, most likely the ones that
bring it closer to a perfect decision tree.

This commit specifically is rather large mostly NFC commit that does
necessary preparation work and refactoring, there will be a following
series of small patches introducing a specific optimization each
shortly after.

This commit specifically is expected to cause a small compile time
regression (around 2.5% of InstructionSelect pass time), which should
be fixed by the next commit of the series.

Every commit planned shares the same Phabricator Review.

Reviewers: qcolombet, dsanders, bogner, aemerson, javed.absar

Reviewed By: qcolombet

Subscribers: rovka, llvm-commits, kristof.beyls

Differential Revision: https://reviews.llvm.org/D44700


git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@332907 91177308-0d34-0410-b5e6-96231b3b80d8
  • Loading branch information
@ramntry
ramntry committed May 21, 2018
1 parent 6a15b19 commit 6e0e075
Show file tree
Hide file tree
Showing 4 changed files with 1,251 additions and 1,049 deletions.
25 changes: 24 additions & 1 deletion include/llvm/CodeGen/GlobalISel/InstructionSelector.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
#include "llvm/ADT/Optional.h"
#include "llvm/ADT/SmallVector.h"
#include "llvm/Support/CodeGenCoverage.h"
#include "llvm/Support/LowLevelTypeImpl.h"
#include <bitset>
#include <cstddef>
#include <cstdint>
Expand All @@ -31,7 +32,6 @@ namespace llvm {

class APInt;
class APFloat;
class LLT;
class MachineInstr;
class MachineInstrBuilder;
class MachineFunction;
Expand Down Expand Up @@ -146,12 +146,14 @@ enum {
/// - OpIdx - Operand index
/// - Expected register bank (specified as a register class)
GIM_CheckRegBankForClass,

/// Check the operand matches a complex predicate
/// - InsnID - Instruction ID
/// - OpIdx - Operand index
/// - RendererID - The renderer to hold the result
/// - Complex predicate ID
GIM_CheckComplexPattern,

/// Check the operand is a specific integer
/// - InsnID - Instruction ID
/// - OpIdx - Operand index
Expand All @@ -168,6 +170,7 @@ enum {
/// - OpIdx - Operand index
/// - Expected Intrinsic ID
GIM_CheckIntrinsicID,

/// Check the specified operand is an MBB
/// - InsnID - Instruction ID
/// - OpIdx - Operand index
Expand Down Expand Up @@ -196,6 +199,7 @@ enum {
/// - OldInsnID - Instruction ID to mutate
/// - NewOpcode - The new opcode to use
GIR_MutateOpcode,

/// Build a new instruction
/// - InsnID - Instruction ID to define
/// - Opcode - The new opcode to use
Expand All @@ -206,6 +210,7 @@ enum {
/// - OldInsnID - Instruction ID to copy from
/// - OpIdx - The operand to copy
GIR_Copy,

/// Copy an operand to the specified instruction or add a zero register if the
/// operand is a zero immediate.
/// - NewInsnID - Instruction ID to modify
Expand All @@ -219,6 +224,7 @@ enum {
/// - OpIdx - The operand to copy
/// - SubRegIdx - The subregister to copy
GIR_CopySubReg,

/// Add an implicit register def to the specified instruction
/// - InsnID - Instruction ID to modify
/// - RegNum - The register to add
Expand All @@ -231,11 +237,13 @@ enum {
/// - InsnID - Instruction ID to modify
/// - RegNum - The register to add
GIR_AddRegister,

/// Add a temporary register to the specified instruction
/// - InsnID - Instruction ID to modify
/// - TempRegID - The temporary register ID to add
/// - TempRegFlags - The register flags to set
GIR_AddTempRegister,

/// Add an immediate to the specified instruction
/// - InsnID - Instruction ID to modify
/// - Imm - The immediate to add
Expand All @@ -244,6 +252,7 @@ enum {
/// - InsnID - Instruction ID to modify
/// - RendererID - The renderer to call
GIR_ComplexRenderer,

/// Render sub-operands of complex operands to the specified instruction
/// - InsnID - Instruction ID to modify
/// - RendererID - The renderer to call
Expand Down Expand Up @@ -272,19 +281,23 @@ enum {
/// - OpIdx - Operand index
/// - RCEnum - Register class enumeration value
GIR_ConstrainOperandRC,

/// Constrain an instructions operands according to the instruction
/// description.
/// - InsnID - Instruction ID to modify
GIR_ConstrainSelectedInstOperands,

/// Merge all memory operands into instruction.
/// - InsnID - Instruction ID to modify
/// - MergeInsnID... - One or more Instruction ID to merge into the result.
/// - GIU_MergeMemOperands_EndOfList - Terminates the list of instructions to
/// merge.
GIR_MergeMemOperands,

/// Erase from parent.
/// - InsnID - Instruction ID to erase
GIR_EraseFromParent,

/// Create a new temporary register that's not constrained.
/// - TempRegID - The temporary register ID to initialize.
/// - Expected type
Expand All @@ -297,6 +310,7 @@ enum {
/// - RuleID - The ID of the rule that was covered.
GIR_Coverage,

/// Keeping track of the number of the GI opcodes. Must be the last entry.
GIU_NumOpcodes,
};

Expand Down Expand Up @@ -341,6 +355,15 @@ class InstructionSelector {
template <class PredicateBitset, class ComplexMatcherMemFn,
class CustomRendererFn>
struct ISelInfoTy {
ISelInfoTy(const LLT *TypeObjects, size_t NumTypeObjects,
const PredicateBitset *FeatureBitsets,
const ComplexMatcherMemFn *ComplexPredicates,
const CustomRendererFn *CustomRenderers)
: TypeObjects(TypeObjects),
FeatureBitsets(FeatureBitsets),
ComplexPredicates(ComplexPredicates),
CustomRenderers(CustomRenderers) {
}
const LLT *TypeObjects;
const PredicateBitset *FeatureBitsets;
const ComplexMatcherMemFn *ComplexPredicates;
Expand Down
22 changes: 9 additions & 13 deletions include/llvm/CodeGen/GlobalISel/InstructionSelectorImpl.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -53,17 +53,17 @@ bool InstructionSelector::executeMatchTable(
MachineRegisterInfo &MRI, const TargetRegisterInfo &TRI,
const RegisterBankInfo &RBI, const PredicateBitset &AvailableFeatures,
CodeGenCoverage &CoverageInfo) const {

uint64_t CurrentIdx = 0;
SmallVector<uint64_t, 8> OnFailResumeAt;
SmallVector<uint64_t, 4> OnFailResumeAt;

enum RejectAction { RejectAndGiveUp, RejectAndResume };
auto handleReject = [&]() -> RejectAction {
DEBUG_WITH_TYPE(TgtInstructionSelector::getName(),
dbgs() << CurrentIdx << ": Rejected\n");
if (OnFailResumeAt.empty())
return RejectAndGiveUp;
CurrentIdx = OnFailResumeAt.back();
OnFailResumeAt.pop_back();
CurrentIdx = OnFailResumeAt.pop_back_val();
DEBUG_WITH_TYPE(TgtInstructionSelector::getName(),
dbgs() << CurrentIdx << ": Resume at " << CurrentIdx << " ("
<< OnFailResumeAt.size() << " try-blocks remain)\n");
Expand Down Expand Up @@ -139,12 +139,13 @@ bool InstructionSelector::executeMatchTable(
int64_t InsnID = MatchTable[CurrentIdx++];
int64_t Expected = MatchTable[CurrentIdx++];

assert(State.MIs[InsnID] != nullptr && "Used insn before defined");
unsigned Opcode = State.MIs[InsnID]->getOpcode();

DEBUG_WITH_TYPE(TgtInstructionSelector::getName(),
dbgs() << CurrentIdx << ": GIM_CheckOpcode(MIs[" << InsnID
<< "], ExpectedOpcode=" << Expected
<< ") // Got=" << Opcode << "\n");
assert(State.MIs[InsnID] != nullptr && "Used insn before defined");
if (Opcode != Expected) {
if (handleReject() == RejectAndGiveUp)
return false;
Expand Down Expand Up @@ -197,7 +198,8 @@ bool InstructionSelector::executeMatchTable(
<< CurrentIdx << ": GIM_CheckAPIntImmPredicate(MIs["
<< InsnID << "], Predicate=" << Predicate << ")\n");
assert(State.MIs[InsnID] != nullptr && "Used insn before defined");
assert(State.MIs[InsnID]->getOpcode() && "Expected G_CONSTANT");
assert(State.MIs[InsnID]->getOpcode() == TargetOpcode::G_CONSTANT &&
"Expected G_CONSTANT");
assert(Predicate > GIPFP_APInt_Invalid && "Expected a valid predicate");
APInt Value;
if (State.MIs[InsnID]->getOperand(1).isCImm())
Expand Down Expand Up @@ -236,7 +238,6 @@ bool InstructionSelector::executeMatchTable(
dbgs() << CurrentIdx << ": GIM_CheckAtomicOrdering(MIs["
<< InsnID << "], " << (uint64_t)Ordering << ")\n");
assert(State.MIs[InsnID] != nullptr && "Used insn before defined");

if (!State.MIs[InsnID]->hasOneMemOperand())
if (handleReject() == RejectAndGiveUp)
return false;
Expand All @@ -255,7 +256,6 @@ bool InstructionSelector::executeMatchTable(
<< ": GIM_CheckAtomicOrderingOrStrongerThan(MIs["
<< InsnID << "], " << (uint64_t)Ordering << ")\n");
assert(State.MIs[InsnID] != nullptr && "Used insn before defined");

if (!State.MIs[InsnID]->hasOneMemOperand())
if (handleReject() == RejectAndGiveUp)
return false;
Expand All @@ -274,7 +274,6 @@ bool InstructionSelector::executeMatchTable(
<< ": GIM_CheckAtomicOrderingWeakerThan(MIs["
<< InsnID << "], " << (uint64_t)Ordering << ")\n");
assert(State.MIs[InsnID] != nullptr && "Used insn before defined");

if (!State.MIs[InsnID]->hasOneMemOperand())
if (handleReject() == RejectAndGiveUp)
return false;
Expand Down Expand Up @@ -375,7 +374,6 @@ bool InstructionSelector::executeMatchTable(
<< "]->getOperand(" << OpIdx
<< "), TypeID=" << TypeID << ")\n");
assert(State.MIs[InsnID] != nullptr && "Used insn before defined");

MachineOperand &MO = State.MIs[InsnID]->getOperand(OpIdx);
if (!MO.isReg() ||
MRI.getType(MO.getReg()) != ISelInfo.TypeObjects[TypeID]) {
Expand All @@ -394,7 +392,6 @@ bool InstructionSelector::executeMatchTable(
<< InsnID << "]->getOperand(" << OpIdx
<< "), SizeInBits=" << SizeInBits << ")\n");
assert(State.MIs[InsnID] != nullptr && "Used insn before defined");

// iPTR must be looked up in the target.
if (SizeInBits == 0) {
MachineFunction *MF = State.MIs[InsnID]->getParent()->getParent();
Expand Down Expand Up @@ -466,7 +463,6 @@ bool InstructionSelector::executeMatchTable(
<< InsnID << "]->getOperand(" << OpIdx
<< "), Value=" << Value << ")\n");
assert(State.MIs[InsnID] != nullptr && "Used insn before defined");

MachineOperand &MO = State.MIs[InsnID]->getOperand(OpIdx);
if (MO.isReg()) {
// isOperandImmEqual() will sign-extend to 64-bits, so should we.
Expand Down Expand Up @@ -562,7 +558,7 @@ bool InstructionSelector::executeMatchTable(
}
case GIM_Reject:
DEBUG_WITH_TYPE(TgtInstructionSelector::getName(),
dbgs() << CurrentIdx << ": GIM_Reject");
dbgs() << CurrentIdx << ": GIM_Reject\n");
if (handleReject() == RejectAndGiveUp)
return false;
break;
Expand Down Expand Up @@ -854,7 +850,7 @@ bool InstructionSelector::executeMatchTable(

case GIR_Done:
DEBUG_WITH_TYPE(TgtInstructionSelector::getName(),
dbgs() << CurrentIdx << ": GIR_Done");
dbgs() << CurrentIdx << ": GIR_Done\n");
return true;

default:
Expand Down
Loading

0 comments on commit 6e0e075

Please sign in to comment.