[llvm] [GlobalISel] Filter combiner worklists (PR #197693)
via llvm-commits
llvm-commits at lists.llvm.org
Thu May 14 07:20:14 PDT 2026
llvmorg-github-actions[bot] wrote:
<!--LLVM PR SUMMARY COMMENT-->
@llvm/pr-subscribers-llvm-globalisel
Author: Cullen Rhodes (c-rhodes)
<details>
<summary>Changes</summary>
This follows on from #<!-- -->196017 which added an opcode predicate for GICombiner
matchers and used it to return from tryCombineAll before executing the match
table.
The better approach is to not add opcodes with no combines to the worklist in
the first place. It's mostly handled by the generated canMatchOpcode but for
combiners with handwritten combines we must also ensure these opcodes get added
to the worklist.
This is a further -0.27% CTMark geomean improvement on aarch64-O0-g, sqlite is
-0.63%.
https://llvm-compile-time-tracker.com/compare.php?from=35f5d7ea802eae78b26a5fb2a46f072acd15f49d&to=c356bec46b68b59f37b26347f93676c9102d810c&stat=instructions%3Au
I also measured O3 locally and it's positive:
```
stage1-aarch64-O3 -fglobal-isel
instructions:u diff
old new
7zip 203863583445 203865335443 0.00%
Bullet 103920943623 103917036315 -0.00%
ClamAV 53008970261 52971646424 -0.07%
SPASS 41843361245 41815471487 -0.07%
consumer-typeset 31848080935 31816255572 -0.10%
kimwitu++ 39779892082 39733563964 -0.12%
lencod 67158153709 67134337444 -0.04%
mafft 36689364945 36675438948 -0.04%
sqlite3 33436727535 33410098701 -0.08%
tramp3d-v4 76553056559 76569825676 0.02%
geomean 57208304686 57180409869 -0.05%
```
I am a bit worried about the incurred overhead of this for other opt levels as
we add more combines, but right now at least the data is positive.
Assisted-by: codex
---
Full diff: https://github.com/llvm/llvm-project/pull/197693.diff
6 Files Affected:
- (modified) llvm/include/llvm/CodeGen/GlobalISel/Combiner.h (+7)
- (modified) llvm/lib/CodeGen/GlobalISel/Combiner.cpp (+2-1)
- (modified) llvm/lib/Target/AArch64/GISel/AArch64O0PreLegalizerCombiner.cpp (+17)
- (modified) llvm/lib/Target/AArch64/GISel/AArch64PreLegalizerCombiner.cpp (+17)
- (modified) llvm/test/TableGen/GlobalISelCombinerEmitter/match-table.td (+1-3)
- (modified) llvm/utils/TableGen/GlobalISelCombinerEmitter.cpp (+6-4)
``````````diff
diff --git a/llvm/include/llvm/CodeGen/GlobalISel/Combiner.h b/llvm/include/llvm/CodeGen/GlobalISel/Combiner.h
index 8b028cd5c023c..b51f7bf17bb3a 100644
--- a/llvm/include/llvm/CodeGen/GlobalISel/Combiner.h
+++ b/llvm/include/llvm/CodeGen/GlobalISel/Combiner.h
@@ -65,6 +65,13 @@ class Combiner : public GIMatchTableExecutor {
bool combineMachineInstrs();
protected:
+ virtual bool canMatchOpcode(unsigned Opc) const { return true; }
+
+ /// Return false to skip adding \p MI to the initial worklist.
+ virtual bool shouldAddToWorkList(MachineInstr &MI) const {
+ return canMatchOpcode(MI.getOpcode());
+ }
+
const CombinerInfo &CInfo;
GISelChangeObserver &Observer;
MachineIRBuilder &B;
diff --git a/llvm/lib/CodeGen/GlobalISel/Combiner.cpp b/llvm/lib/CodeGen/GlobalISel/Combiner.cpp
index a4121fbb7b6c8..f492b9d668160 100644
--- a/llvm/lib/CodeGen/GlobalISel/Combiner.cpp
+++ b/llvm/lib/CodeGen/GlobalISel/Combiner.cpp
@@ -297,7 +297,8 @@ bool Combiner::combineMachineInstrs() {
// Erase dead insts before even adding to the list.
if (EnableDCE && tryDCE(CurMI, MRI))
continue;
- WorkList.deferred_insert(&CurMI);
+ if (shouldAddToWorkList(CurMI))
+ WorkList.deferred_insert(&CurMI);
}
}
WorkList.finalize();
diff --git a/llvm/lib/Target/AArch64/GISel/AArch64O0PreLegalizerCombiner.cpp b/llvm/lib/Target/AArch64/GISel/AArch64O0PreLegalizerCombiner.cpp
index 3970ac530d633..ee9cc36fcddcd 100644
--- a/llvm/lib/Target/AArch64/GISel/AArch64O0PreLegalizerCombiner.cpp
+++ b/llvm/lib/Target/AArch64/GISel/AArch64O0PreLegalizerCombiner.cpp
@@ -65,6 +65,8 @@ class AArch64O0PreLegalizerCombinerImpl : public Combiner {
bool tryCombineAllImpl(MachineInstr &I) const;
+ bool shouldAddToWorkList(MachineInstr &MI) const override;
+
private:
#define GET_GICOMBINER_CLASS_MEMBERS
#include "AArch64GenO0PreLegalizeGICombiner.inc"
@@ -88,6 +90,21 @@ AArch64O0PreLegalizerCombinerImpl::AArch64O0PreLegalizerCombinerImpl(
{
}
+bool AArch64O0PreLegalizerCombinerImpl::shouldAddToWorkList(
+ MachineInstr &MI) const {
+ switch (MI.getOpcode()) {
+ // Keep this in sync with the handwritten opcodes in tryCombineAll().
+ case TargetOpcode::G_SHUFFLE_VECTOR:
+ case TargetOpcode::G_MEMCPY_INLINE:
+ case TargetOpcode::G_MEMCPY:
+ case TargetOpcode::G_MEMMOVE:
+ case TargetOpcode::G_MEMSET:
+ return true;
+ default:
+ return canMatchOpcode(MI.getOpcode());
+ }
+}
+
bool AArch64O0PreLegalizerCombinerImpl::tryCombineAll(MachineInstr &MI) const {
if (tryCombineAllImpl(MI))
return true;
diff --git a/llvm/lib/Target/AArch64/GISel/AArch64PreLegalizerCombiner.cpp b/llvm/lib/Target/AArch64/GISel/AArch64PreLegalizerCombiner.cpp
index 818f8c0c5f277..fdc2c8a72b656 100644
--- a/llvm/lib/Target/AArch64/GISel/AArch64PreLegalizerCombiner.cpp
+++ b/llvm/lib/Target/AArch64/GISel/AArch64PreLegalizerCombiner.cpp
@@ -750,6 +750,7 @@ class AArch64PreLegalizerCombinerImpl : public Combiner {
bool tryCombineAll(MachineInstr &I) const override;
bool tryCombineAllImpl(MachineInstr &I) const;
+ bool shouldAddToWorkList(MachineInstr &MI) const override;
private:
#define GET_GICOMBINER_CLASS_MEMBERS
@@ -776,6 +777,22 @@ AArch64PreLegalizerCombinerImpl::AArch64PreLegalizerCombinerImpl(
{
}
+bool AArch64PreLegalizerCombinerImpl::shouldAddToWorkList(
+ MachineInstr &MI) const {
+ switch (MI.getOpcode()) {
+ // Keep this in sync with the handwritten opcodes in tryCombineAll().
+ case TargetOpcode::G_SHUFFLE_VECTOR:
+ case TargetOpcode::G_UADDO:
+ case TargetOpcode::G_MEMCPY_INLINE:
+ case TargetOpcode::G_MEMCPY:
+ case TargetOpcode::G_MEMMOVE:
+ case TargetOpcode::G_MEMSET:
+ return true;
+ default:
+ return canMatchOpcode(MI.getOpcode());
+ }
+}
+
bool AArch64PreLegalizerCombinerImpl::tryCombineAll(MachineInstr &MI) const {
if (tryCombineAllImpl(MI))
return true;
diff --git a/llvm/test/TableGen/GlobalISelCombinerEmitter/match-table.td b/llvm/test/TableGen/GlobalISelCombinerEmitter/match-table.td
index 27b1dbc02b2e6..18678b4f40d53 100644
--- a/llvm/test/TableGen/GlobalISelCombinerEmitter/match-table.td
+++ b/llvm/test/TableGen/GlobalISelCombinerEmitter/match-table.td
@@ -84,7 +84,7 @@ def MyCombiner: GICombiner<"GenMyCombiner", [
// Verify we gate on opcodes with generated combines and reset MatchData on each
// tryCombineAll.
-// CHECK: static bool GenMyCombiner_canMatchOpcode(unsigned Opc) {
+// CHECK: bool GenMyCombiner::canMatchOpcode(unsigned Opc) const {
// CHECK-NEXT: switch (Opc) {
// CHECK-DAG: case TargetOpcode::COPY:
// CHECK-DAG: case TargetOpcode::G_AND:
@@ -99,8 +99,6 @@ def MyCombiner: GICombiner<"GenMyCombiner", [
// CHECK-NEXT: }
// CHECK-EMPTY:
// CHECK: bool GenMyCombiner::tryCombineAll(MachineInstr &I) const {
-// CHECK-NEXT: if (!GenMyCombiner_canMatchOpcode(I.getOpcode()))
-// CHECK-NEXT: return false;
// CHECK-NEXT: const PredicateBitset AvailableFeatures = getAvailableFeatures();
// CHECK-NEXT: State.MIs.clear();
// CHECK-NEXT: State.MIs.push_back(&I);
diff --git a/llvm/utils/TableGen/GlobalISelCombinerEmitter.cpp b/llvm/utils/TableGen/GlobalISelCombinerEmitter.cpp
index e01f2a2685049..d3cd12fb41909 100644
--- a/llvm/utils/TableGen/GlobalISelCombinerEmitter.cpp
+++ b/llvm/utils/TableGen/GlobalISelCombinerEmitter.cpp
@@ -2573,7 +2573,7 @@ void GICombinerEmitter::collectMatchOpcodes(ArrayRef<RuleMatcher> Rules) {
void GICombinerEmitter::emitCanMatchOpcodeFn(raw_ostream &OS,
StringRef FnName) const {
- OS << "static bool " << FnName << "(unsigned Opc) {\n";
+ OS << "bool " << FnName << "(unsigned Opc) const {\n";
if (MatchOpcodes.empty()) {
OS << " (void)Opc;\n"
<< " return false;\n"
@@ -2592,12 +2592,10 @@ void GICombinerEmitter::emitCanMatchOpcodeFn(raw_ostream &OS,
}
void GICombinerEmitter::emitAdditionalImpl(raw_ostream &OS) {
- std::string CanMatchOpcodeFnName = (getClassName() + "_canMatchOpcode").str();
+ std::string CanMatchOpcodeFnName = (getClassName() + "::canMatchOpcode").str();
emitCanMatchOpcodeFn(OS, CanMatchOpcodeFnName);
OS << "bool " << getClassName() << "::" << getCombineAllMethodName()
<< "(MachineInstr &I) const {\n"
- << " if (!" << CanMatchOpcodeFnName << "(I.getOpcode()))\n"
- << " return false;\n"
<< " const PredicateBitset AvailableFeatures = "
"getAvailableFeatures();\n"
<< " State.MIs.clear();\n"
@@ -2847,6 +2845,10 @@ void GICombinerEmitter::run(raw_ostream &OS) {
emitPredicateBitset(OS, "GET_GICOMBINER_TYPES");
// GET_GICOMBINER_CLASS_MEMBERS, which need to be included inside the class.
+ {
+ IfDefGuardEmitter If(OS, "GET_GICOMBINER_CLASS_MEMBERS");
+ OS << " bool canMatchOpcode(unsigned Opc) const override;\n";
+ }
emitPredicatesDecl(OS, "GET_GICOMBINER_CLASS_MEMBERS");
emitTemporariesDecl(OS, "GET_GICOMBINER_CLASS_MEMBERS");
``````````
</details>
https://github.com/llvm/llvm-project/pull/197693
More information about the llvm-commits
mailing list