Skip to content

[RFC][GlobalISel] Use Builders in MatchTable #65955

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Oct 16, 2023
Merged

[RFC][GlobalISel] Use Builders in MatchTable #65955

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
ec61840
[RFC][GlobalISel] Use Builders in MatchTable
Pierre-vh Sep 11, 2023
06e1fb6
Revert Observer Changes
Pierre-vh Sep 22, 2023
bd43456
clang-format
Pierre-vh Oct 9, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
39 changes: 28 additions & 11 deletions llvm/include/llvm/CodeGen/GlobalISel/GIMatchTableExecutor.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
#include "llvm/ADT/Bitset.h"
#include "llvm/ADT/DenseMap.h"
#include "llvm/ADT/SmallVector.h"
#include "llvm/CodeGen/GlobalISel/GISelChangeObserver.h"
#include "llvm/CodeGen/GlobalISel/Utils.h"
#include "llvm/CodeGen/LowLevelType.h"
#include "llvm/CodeGen/MachineFunction.h"
Expand All @@ -40,6 +41,7 @@ class APInt;
class APFloat;
class GISelKnownBits;
class MachineInstr;
class MachineIRBuilder;
class MachineInstrBuilder;
class MachineFunction;
class MachineOperand;
Expand Down Expand Up @@ -501,10 +503,25 @@ class GIMatchTableExecutor {
}

protected:
/// Observer used by \ref executeMatchTable to record all instructions created
/// by the rule.
class GIMatchTableObserver : public GISelChangeObserver {
public:
virtual ~GIMatchTableObserver();

void erasingInstr(MachineInstr &MI) override { CreatedInsts.erase(&MI); }
void createdInstr(MachineInstr &MI) override { CreatedInsts.insert(&MI); }
void changingInstr(MachineInstr &MI) override {}
void changedInstr(MachineInstr &MI) override {}

// Keeps track of all instructions that have been created when applying a
// rule.
SmallDenseSet<MachineInstr *, 4> CreatedInsts;
};

using ComplexRendererFns =
std::optional<SmallVector<std::function<void(MachineInstrBuilder &)>, 4>>;
using RecordedMIVector = SmallVector<MachineInstr *, 4>;
using NewMIVector = SmallVector<MachineInstrBuilder, 4>;

struct MatcherState {
std::vector<ComplexRendererFns::value_type> Renderers;
Expand Down Expand Up @@ -555,15 +572,15 @@ class GIMatchTableExecutor {
/// and false otherwise.
template <class TgtExecutor, class PredicateBitset, class ComplexMatcherMemFn,
class CustomRendererFn>
bool executeMatchTable(
TgtExecutor &Exec, NewMIVector &OutMIs, MatcherState &State,
const ExecInfoTy<PredicateBitset, ComplexMatcherMemFn, CustomRendererFn>
&ISelInfo,
const int64_t *MatchTable, const TargetInstrInfo &TII,
MachineRegisterInfo &MRI, const TargetRegisterInfo &TRI,
const RegisterBankInfo &RBI, const PredicateBitset &AvailableFeatures,
CodeGenCoverage *CoverageInfo,
GISelChangeObserver *Observer = nullptr) const;
bool executeMatchTable(TgtExecutor &Exec, MatcherState &State,
const ExecInfoTy<PredicateBitset, ComplexMatcherMemFn,
CustomRendererFn> &ExecInfo,
MachineIRBuilder &Builder, const int64_t *MatchTable,
const TargetInstrInfo &TII, MachineRegisterInfo &MRI,
const TargetRegisterInfo &TRI,
const RegisterBankInfo &RBI,
const PredicateBitset &AvailableFeatures,
CodeGenCoverage *CoverageInfo) const;

virtual const int64_t *getMatchTable() const {
llvm_unreachable("Should have been overridden by tablegen if used");
Expand Down Expand Up @@ -592,7 +609,7 @@ class GIMatchTableExecutor {
}

virtual void runCustomAction(unsigned, const MatcherState &State,
NewMIVector &OutMIs) const {
ArrayRef<MachineInstrBuilder> OutMIs) const {
llvm_unreachable("Subclass does not implement runCustomAction!");
}

Expand Down
66 changes: 40 additions & 26 deletions llvm/include/llvm/CodeGen/GlobalISel/GIMatchTableExecutorImpl.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
#include "llvm/ADT/SmallVector.h"
#include "llvm/CodeGen/GlobalISel/GIMatchTableExecutor.h"
#include "llvm/CodeGen/GlobalISel/GISelChangeObserver.h"
#include "llvm/CodeGen/GlobalISel/MachineIRBuilder.h"
#include "llvm/CodeGen/GlobalISel/Utils.h"
#include "llvm/CodeGen/MachineInstrBuilder.h"
#include "llvm/CodeGen/MachineOperand.h"
Expand All @@ -42,17 +43,33 @@ namespace llvm {
template <class TgtExecutor, class PredicateBitset, class ComplexMatcherMemFn,
class CustomRendererFn>
bool GIMatchTableExecutor::executeMatchTable(
TgtExecutor &Exec, NewMIVector &OutMIs, MatcherState &State,
TgtExecutor &Exec, MatcherState &State,
const ExecInfoTy<PredicateBitset, ComplexMatcherMemFn, CustomRendererFn>
&ExecInfo,
const int64_t *MatchTable, const TargetInstrInfo &TII,
MachineRegisterInfo &MRI, const TargetRegisterInfo &TRI,
const RegisterBankInfo &RBI, const PredicateBitset &AvailableFeatures,
CodeGenCoverage *CoverageInfo, GISelChangeObserver *Observer) const {
MachineIRBuilder &Builder, const int64_t *MatchTable,
const TargetInstrInfo &TII, MachineRegisterInfo &MRI,
const TargetRegisterInfo &TRI, const RegisterBankInfo &RBI,
const PredicateBitset &AvailableFeatures,
CodeGenCoverage *CoverageInfo) const {

// Setup observer
GIMatchTableObserver MTObserver;
GISelObserverWrapper Observer(&MTObserver);
if (auto *CurObs = Builder.getObserver())
Observer.addObserver(CurObs);

// TODO: Set MF delegate?

// Setup builder.
auto RestoreOldObserver = Builder.setTemporaryChangeObserver(Observer);

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

// We also record MachineInstrs manually in this vector so opcodes can address
// them.
SmallVector<MachineInstrBuilder, 4> OutMIs;

// Bypass the flag check on the instruction, and only look at the MCInstrDesc.
bool NoFPException = !State.MIs[0]->getDesc().mayRaiseFPException();

Expand All @@ -71,14 +88,16 @@ bool GIMatchTableExecutor::executeMatchTable(
return RejectAndResume;
};

auto propagateFlags = [=](NewMIVector &OutMIs) {
for (auto MIB : OutMIs) {
auto propagateFlags = [&]() {
for (auto *MI : MTObserver.CreatedInsts) {
// Set the NoFPExcept flag when no original matched instruction could
// raise an FP exception, but the new instruction potentially might.
uint16_t MIBFlags = Flags;
if (NoFPException && MIB->mayRaiseFPException())
if (NoFPException && MI->mayRaiseFPException())
MIBFlags |= MachineInstr::NoFPExcept;
MIB.setMIFlags(MIBFlags);
Observer.changingInstr(*MI);
MI->setFlags(MIBFlags);
Observer.changedInstr(*MI);
}

return true;
Expand Down Expand Up @@ -901,6 +920,7 @@ bool GIMatchTableExecutor::executeMatchTable(
OutMIs[NewInsnID] = MachineInstrBuilder(*State.MIs[OldInsnID]->getMF(),
State.MIs[OldInsnID]);
OutMIs[NewInsnID]->setDesc(TII.get(NewOpcode));
MTObserver.CreatedInsts.insert(OutMIs[NewInsnID]);
DEBUG_WITH_TYPE(TgtExecutor::getName(),
dbgs() << CurrentIdx << ": GIR_MutateOpcode(OutMIs["
<< NewInsnID << "], MIs[" << OldInsnID << "], "
Expand All @@ -914,8 +934,7 @@ bool GIMatchTableExecutor::executeMatchTable(
if (NewInsnID >= OutMIs.size())
OutMIs.resize(NewInsnID + 1);

OutMIs[NewInsnID] = BuildMI(*State.MIs[0]->getParent(), State.MIs[0],
MIMetadata(*State.MIs[0]), TII.get(Opcode));
OutMIs[NewInsnID] = Builder.buildInstr(Opcode);
DEBUG_WITH_TYPE(TgtExecutor::getName(),
dbgs() << CurrentIdx << ": GIR_BuildMI(OutMIs["
<< NewInsnID << "], " << Opcode << ")\n");
Expand Down Expand Up @@ -1239,8 +1258,11 @@ bool GIMatchTableExecutor::executeMatchTable(
DEBUG_WITH_TYPE(TgtExecutor::getName(),
dbgs() << CurrentIdx << ": GIR_EraseFromParent(MIs["
<< InsnID << "])\n");
if (Observer)
Observer->erasingInstr(*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());
Observer.erasingInstr(*MI);
MI->eraseFromParent();
break;
}
Expand Down Expand Up @@ -1269,11 +1291,9 @@ bool GIMatchTableExecutor::executeMatchTable(

Register Old = State.MIs[OldInsnID]->getOperand(OldOpIdx).getReg();
Register New = State.MIs[NewInsnID]->getOperand(NewOpIdx).getReg();
if (Observer)
Observer->changingAllUsesOfReg(MRI, Old);
Observer.changingAllUsesOfReg(MRI, Old);
MRI.replaceRegWith(Old, New);
if (Observer)
Observer->finishedChangingAllUsesOfReg();
Observer.finishedChangingAllUsesOfReg();
break;
}
case GIR_ReplaceRegWithTempReg: {
Expand All @@ -1288,11 +1308,9 @@ bool GIMatchTableExecutor::executeMatchTable(

Register Old = State.MIs[OldInsnID]->getOperand(OldOpIdx).getReg();
Register New = State.TempRegisters[TempRegID];
if (Observer)
Observer->changingAllUsesOfReg(MRI, Old);
Observer.changingAllUsesOfReg(MRI, Old);
MRI.replaceRegWith(Old, New);
if (Observer)
Observer->finishedChangingAllUsesOfReg();
Observer.finishedChangingAllUsesOfReg();
break;
}
case GIR_Coverage: {
Expand All @@ -1309,11 +1327,7 @@ bool GIMatchTableExecutor::executeMatchTable(
case GIR_Done:
DEBUG_WITH_TYPE(TgtExecutor::getName(),
dbgs() << CurrentIdx << ": GIR_Done\n");
if (Observer) {
for (MachineInstr *MI : OutMIs)
Observer->createdInstr(*MI);
}
propagateFlags(OutMIs);
propagateFlags();
return true;
default:
llvm_unreachable("Unexpected command");
Expand Down
8 changes: 8 additions & 0 deletions llvm/include/llvm/CodeGen/GlobalISel/MachineIRBuilder.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
#include "llvm/CodeGen/TargetOpcodes.h"
#include "llvm/IR/DebugLoc.h"
#include "llvm/IR/Module.h"
#include "llvm/Support/SaveAndRestore.h"

namespace llvm {

Expand Down Expand Up @@ -364,6 +365,13 @@ class MachineIRBuilder {
State.Observer = &Observer;
}

// Replaces the change observer with \p Observer and returns an object that
// restores the old Observer on destruction.
SaveAndRestore<GISelChangeObserver *>
setTemporaryChangeObserver(GISelChangeObserver &Observer) {
return SaveAndRestore<GISelChangeObserver *>(State.Observer, &Observer);
}

GISelChangeObserver *getObserver() { return State.Observer; }

void stopObservingChanges() { State.Observer = nullptr; }
Expand Down
4 changes: 4 additions & 0 deletions llvm/lib/CodeGen/GlobalISel/GIMatchTableExecutor.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,10 @@

using namespace llvm;

GIMatchTableExecutor::GIMatchTableObserver::~GIMatchTableObserver() {
// anchor
}

GIMatchTableExecutor::MatcherState::MatcherState(unsigned MaxRenderers)
: Renderers(MaxRenderers) {}

Expand Down
28 changes: 14 additions & 14 deletions llvm/test/CodeGen/AArch64/GlobalISel/combine-sdiv.mir
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,10 +22,10 @@ body: |
; CHECK: liveins: $w0
; CHECK-NEXT: {{ $}}
; CHECK-NEXT: [[COPY:%[0-9]+]]:_(s32) = COPY $w0
; CHECK-NEXT: [[C:%[0-9]+]]:_(s32) = G_CONSTANT i32 3
; CHECK-NEXT: [[C1:%[0-9]+]]:_(s32) = G_CONSTANT i32 -991146299
; CHECK-NEXT: [[C:%[0-9]+]]:_(s32) = exact G_CONSTANT i32 3
; CHECK-NEXT: [[C1:%[0-9]+]]:_(s32) = exact G_CONSTANT i32 -991146299
; CHECK-NEXT: [[ASHR:%[0-9]+]]:_(s32) = exact G_ASHR [[COPY]], [[C]](s32)
; CHECK-NEXT: [[MUL:%[0-9]+]]:_(s32) = G_MUL [[ASHR]], [[C1]]
; CHECK-NEXT: [[MUL:%[0-9]+]]:_(s32) = exact G_MUL [[ASHR]], [[C1]]
; CHECK-NEXT: $w0 = COPY [[MUL]](s32)
; CHECK-NEXT: RET_ReallyLR implicit $w0
%0:_(s32) = COPY $w0
Expand Down Expand Up @@ -87,13 +87,13 @@ body: |
; CHECK: liveins: $q0
; CHECK-NEXT: {{ $}}
; CHECK-NEXT: [[COPY:%[0-9]+]]:_(<4 x s32>) = COPY $q0
; CHECK-NEXT: [[C:%[0-9]+]]:_(s32) = G_CONSTANT i32 3
; CHECK-NEXT: [[C1:%[0-9]+]]:_(s32) = G_CONSTANT i32 -991146299
; CHECK-NEXT: [[C2:%[0-9]+]]:_(s32) = G_CONSTANT i32 954437177
; CHECK-NEXT: [[BUILD_VECTOR:%[0-9]+]]:_(<4 x s32>) = G_BUILD_VECTOR [[C]](s32), [[C]](s32), [[C]](s32), [[C]](s32)
; CHECK-NEXT: [[BUILD_VECTOR1:%[0-9]+]]:_(<4 x s32>) = G_BUILD_VECTOR [[C1]](s32), [[C2]](s32), [[C1]](s32), [[C2]](s32)
; CHECK-NEXT: [[C:%[0-9]+]]:_(s32) = exact G_CONSTANT i32 3
; CHECK-NEXT: [[C1:%[0-9]+]]:_(s32) = exact G_CONSTANT i32 -991146299
; CHECK-NEXT: [[C2:%[0-9]+]]:_(s32) = exact G_CONSTANT i32 954437177
; CHECK-NEXT: [[BUILD_VECTOR:%[0-9]+]]:_(<4 x s32>) = exact G_BUILD_VECTOR [[C]](s32), [[C]](s32), [[C]](s32), [[C]](s32)
; CHECK-NEXT: [[BUILD_VECTOR1:%[0-9]+]]:_(<4 x s32>) = exact G_BUILD_VECTOR [[C1]](s32), [[C2]](s32), [[C1]](s32), [[C2]](s32)
; CHECK-NEXT: [[ASHR:%[0-9]+]]:_(<4 x s32>) = exact G_ASHR [[COPY]], [[BUILD_VECTOR]](<4 x s32>)
; CHECK-NEXT: [[MUL:%[0-9]+]]:_(<4 x s32>) = G_MUL [[ASHR]], [[BUILD_VECTOR1]]
; CHECK-NEXT: [[MUL:%[0-9]+]]:_(<4 x s32>) = exact G_MUL [[ASHR]], [[BUILD_VECTOR1]]
; CHECK-NEXT: $q0 = COPY [[MUL]](<4 x s32>)
; CHECK-NEXT: RET_ReallyLR implicit $q0
%0:_(<4 x s32>) = COPY $q0
Expand All @@ -115,12 +115,12 @@ body: |
; CHECK: liveins: $q0
; CHECK-NEXT: {{ $}}
; CHECK-NEXT: [[COPY:%[0-9]+]]:_(<4 x s32>) = COPY $q0
; CHECK-NEXT: [[C:%[0-9]+]]:_(s32) = G_CONSTANT i32 3
; CHECK-NEXT: [[C1:%[0-9]+]]:_(s32) = G_CONSTANT i32 -991146299
; CHECK-NEXT: [[BUILD_VECTOR:%[0-9]+]]:_(<4 x s32>) = G_BUILD_VECTOR [[C]](s32), [[C]](s32), [[C]](s32), [[C]](s32)
; CHECK-NEXT: [[BUILD_VECTOR1:%[0-9]+]]:_(<4 x s32>) = G_BUILD_VECTOR [[C1]](s32), [[C1]](s32), [[C1]](s32), [[C1]](s32)
; CHECK-NEXT: [[C:%[0-9]+]]:_(s32) = exact G_CONSTANT i32 3
; CHECK-NEXT: [[C1:%[0-9]+]]:_(s32) = exact G_CONSTANT i32 -991146299
; CHECK-NEXT: [[BUILD_VECTOR:%[0-9]+]]:_(<4 x s32>) = exact G_BUILD_VECTOR [[C]](s32), [[C]](s32), [[C]](s32), [[C]](s32)
; CHECK-NEXT: [[BUILD_VECTOR1:%[0-9]+]]:_(<4 x s32>) = exact G_BUILD_VECTOR [[C1]](s32), [[C1]](s32), [[C1]](s32), [[C1]](s32)
; CHECK-NEXT: [[ASHR:%[0-9]+]]:_(<4 x s32>) = exact G_ASHR [[COPY]], [[BUILD_VECTOR]](<4 x s32>)
; CHECK-NEXT: [[MUL:%[0-9]+]]:_(<4 x s32>) = G_MUL [[ASHR]], [[BUILD_VECTOR1]]
; CHECK-NEXT: [[MUL:%[0-9]+]]:_(<4 x s32>) = exact G_MUL [[ASHR]], [[BUILD_VECTOR1]]
; CHECK-NEXT: $q0 = COPY [[MUL]](<4 x s32>)
; CHECK-NEXT: RET_ReallyLR implicit $q0
%0:_(<4 x s32>) = COPY $q0
Expand Down
4 changes: 2 additions & 2 deletions llvm/test/CodeGen/AArch64/GlobalISel/prelegalizercombiner-ashr-shl-to-sext-inreg.mir
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ body: |
; CHECK-NEXT: {{ $}}
; CHECK-NEXT: [[COPY:%[0-9]+]]:_(s32) = COPY $w0
; CHECK-NEXT: [[TRUNC:%[0-9]+]]:_(s16) = G_TRUNC [[COPY]](s32)
; CHECK-NEXT: [[SEXT_INREG:%[0-9]+]]:_(s16) = G_SEXT_INREG [[TRUNC]], 8
; CHECK-NEXT: [[SEXT_INREG:%[0-9]+]]:_(s16) = exact G_SEXT_INREG [[TRUNC]], 8
; CHECK-NEXT: [[ANYEXT:%[0-9]+]]:_(s32) = G_ANYEXT [[SEXT_INREG]](s16)
; CHECK-NEXT: $w0 = COPY [[ANYEXT]](s32)
; CHECK-NEXT: RET_ReallyLR implicit $w0
Expand Down Expand Up @@ -75,7 +75,7 @@ body: |
; CHECK: liveins: $d0
; CHECK-NEXT: {{ $}}
; CHECK-NEXT: [[COPY:%[0-9]+]]:_(<4 x s16>) = COPY $d0
; CHECK-NEXT: [[SEXT_INREG:%[0-9]+]]:_(<4 x s16>) = G_SEXT_INREG [[COPY]], 8
; CHECK-NEXT: [[SEXT_INREG:%[0-9]+]]:_(<4 x s16>) = exact G_SEXT_INREG [[COPY]], 8
; CHECK-NEXT: $d0 = COPY [[SEXT_INREG]](<4 x s16>)
; CHECK-NEXT: RET_ReallyLR implicit $d0
%0:_(<4 x s16>) = COPY $d0
Expand Down
Loading