[llvm] 96e473a - [RFC][GlobalISel] Use Builders in MatchTable (#65955)
via llvm-commits
llvm-commits at lists.llvm.org
Sun Oct 15 22:41:23 PDT 2023
Author: Pierre van Houtryve
Date: 2023-10-16T07:41:18+02:00
New Revision: 96e473a6be2e82e3fb4060805c7928c981111025
URL: https://github.com/llvm/llvm-project/commit/96e473a6be2e82e3fb4060805c7928c981111025
DIFF: https://github.com/llvm/llvm-project/commit/96e473a6be2e82e3fb4060805c7928c981111025.diff
LOG: [RFC][GlobalISel] Use Builders in MatchTable (#65955)
The MatchTableExecutor did not use the MachineIRBuilder but instead
created instructions ad-hoc.
Making it use a Builder has the benefit that any observer added by a
combine is now notified when instructions are created by MIR patterns.
Another benefit is that it allows me to improve how constants are
created in apply MIR patterns.
`MachineIRBuilder::buildConstant` automatically handles splats for us,
this means that we may change `addCImm` to use that and handle vector
cases automatically.
Added:
Modified:
llvm/include/llvm/CodeGen/GlobalISel/GIMatchTableExecutor.h
llvm/include/llvm/CodeGen/GlobalISel/GIMatchTableExecutorImpl.h
llvm/test/TableGen/GlobalISelCombinerEmitter/match-table.td
llvm/test/TableGen/GlobalISelEmitter.td
llvm/utils/TableGen/GlobalISelCombinerEmitter.cpp
llvm/utils/TableGen/GlobalISelEmitter.cpp
Removed:
################################################################################
diff --git a/llvm/include/llvm/CodeGen/GlobalISel/GIMatchTableExecutor.h b/llvm/include/llvm/CodeGen/GlobalISel/GIMatchTableExecutor.h
index 2b0733cf9353e6c..45da6d96aa3de2b 100644
--- a/llvm/include/llvm/CodeGen/GlobalISel/GIMatchTableExecutor.h
+++ b/llvm/include/llvm/CodeGen/GlobalISel/GIMatchTableExecutor.h
@@ -40,6 +40,7 @@ class APInt;
class APFloat;
class GISelKnownBits;
class MachineInstr;
+class MachineIRBuilder;
class MachineInstrBuilder;
class MachineFunction;
class MachineOperand;
@@ -555,15 +556,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");
diff --git a/llvm/include/llvm/CodeGen/GlobalISel/GIMatchTableExecutorImpl.h b/llvm/include/llvm/CodeGen/GlobalISel/GIMatchTableExecutorImpl.h
index 883c1ca0fe350b0..6f0f9a6a46c7cef 100644
--- a/llvm/include/llvm/CodeGen/GlobalISel/GIMatchTableExecutorImpl.h
+++ b/llvm/include/llvm/CodeGen/GlobalISel/GIMatchTableExecutorImpl.h
@@ -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"
@@ -42,17 +43,20 @@ 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 {
uint64_t CurrentIdx = 0;
SmallVector<uint64_t, 4> OnFailResumeAt;
+ NewMIVector OutMIs;
+ GISelChangeObserver *Observer = Builder.getObserver();
// Bypass the flag check on the instruction, and only look at the MCInstrDesc.
bool NoFPException = !State.MIs[0]->getDesc().mayRaiseFPException();
@@ -71,14 +75,18 @@ bool GIMatchTableExecutor::executeMatchTable(
return RejectAndResume;
};
- auto propagateFlags = [=](NewMIVector &OutMIs) {
+ const auto propagateFlags = [&]() {
for (auto MIB : OutMIs) {
// 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())
MIBFlags |= MachineInstr::NoFPExcept;
+ if (Observer)
+ Observer->changingInstr(*MIB);
MIB.setMIFlags(MIBFlags);
+ if (Observer)
+ Observer->changedInstr(*MIB);
}
return true;
@@ -898,9 +906,13 @@ bool GIMatchTableExecutor::executeMatchTable(
if (NewInsnID >= OutMIs.size())
OutMIs.resize(NewInsnID + 1);
- OutMIs[NewInsnID] = MachineInstrBuilder(*State.MIs[OldInsnID]->getMF(),
- State.MIs[OldInsnID]);
+ MachineInstr *OldMI = State.MIs[OldInsnID];
+ if (Observer)
+ Observer->changingInstr(*OldMI);
+ OutMIs[NewInsnID] = MachineInstrBuilder(*OldMI->getMF(), OldMI);
OutMIs[NewInsnID]->setDesc(TII.get(NewOpcode));
+ if (Observer)
+ Observer->changedInstr(*OldMI);
DEBUG_WITH_TYPE(TgtExecutor::getName(),
dbgs() << CurrentIdx << ": GIR_MutateOpcode(OutMIs["
<< NewInsnID << "], MIs[" << OldInsnID << "], "
@@ -914,8 +926,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");
@@ -1239,6 +1250,10 @@ bool GIMatchTableExecutor::executeMatchTable(
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();
@@ -1309,11 +1324,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");
diff --git a/llvm/test/TableGen/GlobalISelCombinerEmitter/match-table.td b/llvm/test/TableGen/GlobalISelCombinerEmitter/match-table.td
index b810c519d2ac36f..f51a18c4d3e7329 100644
--- a/llvm/test/TableGen/GlobalISelCombinerEmitter/match-table.td
+++ b/llvm/test/TableGen/GlobalISelCombinerEmitter/match-table.td
@@ -93,12 +93,12 @@ def MyCombiner: GICombiner<"GenMyCombiner", [
// CHECK: bool GenMyCombiner::tryCombineAll(MachineInstr &I) const {
// CHECK-NEXT: const TargetSubtargetInfo &ST = MF.getSubtarget();
// CHECK-NEXT: const PredicateBitset AvailableFeatures = getAvailableFeatures();
-// CHECK-NEXT: NewMIVector OutMIs;
+// CHECK-NEXT: B.setInstrAndDebugLoc(I);
// CHECK-NEXT: State.MIs.clear();
// CHECK-NEXT: State.MIs.push_back(&I);
// CHECK-NEXT: MatchInfos = MatchInfosTy();
// CHECK-EMPTY:
-// CHECK-NEXT: if (executeMatchTable(*this, OutMIs, State, ExecInfo, getMatchTable(), *ST.getInstrInfo(), MRI, *MRI.getTargetRegisterInfo(), *ST.getRegBankInfo(), AvailableFeatures, /*CoverageInfo*/ nullptr, &Observer))
+// CHECK-NEXT: if (executeMatchTable(*this, State, ExecInfo, B, getMatchTable(), *ST.getInstrInfo(), MRI, *MRI.getTargetRegisterInfo(), *ST.getRegBankInfo(), AvailableFeatures, /*CoverageInfo*/ nullptr))
// CHECK-NEXT: return true;
// CHECK-NEXT: }
// CHECK-EMPTY:
diff --git a/llvm/test/TableGen/GlobalISelEmitter.td b/llvm/test/TableGen/GlobalISelEmitter.td
index 7cca2d52e4062e8..b7a81894f6442fa 100644
--- a/llvm/test/TableGen/GlobalISelEmitter.td
+++ b/llvm/test/TableGen/GlobalISelEmitter.td
@@ -216,11 +216,11 @@ def HasC : Predicate<"Subtarget->hasC()"> { let RecomputePerFunction = 1; }
// CHECK: bool MyTargetInstructionSelector::selectImpl(MachineInstr &I, CodeGenCoverage &CoverageInfo) const {
// CHECK-NEXT: const PredicateBitset AvailableFeatures = getAvailableFeatures();
-// CHECK-NEXT: NewMIVector OutMIs;
+// CHECK-NEXT: MachineIRBuilder B(I);
// CHECK-NEXT: State.MIs.clear();
// CHECK-NEXT: State.MIs.push_back(&I);
-// CHECK: if (executeMatchTable(*this, OutMIs, State, ExecInfo, getMatchTable(), TII, MF->getRegInfo(), TRI, RBI, AvailableFeatures, &CoverageInfo)) {
+// CHECK: if (executeMatchTable(*this, State, ExecInfo, B, getMatchTable(), TII, MF->getRegInfo(), TRI, RBI, AvailableFeatures, &CoverageInfo)) {
// CHECK-NEXT: return true;
// CHECK-NEXT: }
diff --git a/llvm/utils/TableGen/GlobalISelCombinerEmitter.cpp b/llvm/utils/TableGen/GlobalISelCombinerEmitter.cpp
index b28915148ee5169..809415aeff153f7 100644
--- a/llvm/utils/TableGen/GlobalISelCombinerEmitter.cpp
+++ b/llvm/utils/TableGen/GlobalISelCombinerEmitter.cpp
@@ -3465,15 +3465,15 @@ void GICombinerEmitter::emitAdditionalImpl(raw_ostream &OS) {
<< " const TargetSubtargetInfo &ST = MF.getSubtarget();\n"
<< " const PredicateBitset AvailableFeatures = "
"getAvailableFeatures();\n"
- << " NewMIVector OutMIs;\n"
+ << " B.setInstrAndDebugLoc(I);\n"
<< " State.MIs.clear();\n"
<< " State.MIs.push_back(&I);\n"
<< " " << MatchDataInfo::StructName << " = "
<< MatchDataInfo::StructTypeName << "();\n\n"
- << " if (executeMatchTable(*this, OutMIs, State, ExecInfo"
+ << " if (executeMatchTable(*this, State, ExecInfo, B"
<< ", getMatchTable(), *ST.getInstrInfo(), MRI, "
"*MRI.getTargetRegisterInfo(), *ST.getRegBankInfo(), AvailableFeatures"
- << ", /*CoverageInfo*/ nullptr, &Observer)) {\n"
+ << ", /*CoverageInfo*/ nullptr)) {\n"
<< " return true;\n"
<< " }\n\n"
<< " return false;\n"
diff --git a/llvm/utils/TableGen/GlobalISelEmitter.cpp b/llvm/utils/TableGen/GlobalISelEmitter.cpp
index 2ea48904466af45..8d9ded1b2ac5e9c 100644
--- a/llvm/utils/TableGen/GlobalISelEmitter.cpp
+++ b/llvm/utils/TableGen/GlobalISelEmitter.cpp
@@ -2267,10 +2267,10 @@ void GlobalISelEmitter::emitAdditionalImpl(raw_ostream &OS) {
"&CoverageInfo) const {\n"
<< " const PredicateBitset AvailableFeatures = "
"getAvailableFeatures();\n"
- << " NewMIVector OutMIs;\n"
+ << " MachineIRBuilder B(I);\n"
<< " State.MIs.clear();\n"
<< " State.MIs.push_back(&I);\n\n"
- << " if (executeMatchTable(*this, OutMIs, State, ExecInfo"
+ << " if (executeMatchTable(*this, State, ExecInfo, B"
<< ", getMatchTable(), TII, MF->getRegInfo(), TRI, RBI, AvailableFeatures"
<< ", &CoverageInfo)) {\n"
<< " return true;\n"
More information about the llvm-commits
mailing list