[llvm] [MacroFusion] Early return when insts already clustered (PR #191710)
Tomer Shafir via llvm-commits
llvm-commits at lists.llvm.org
Mon Apr 13 05:11:34 PDT 2026
https://github.com/tomershafir updated https://github.com/llvm/llvm-project/pull/191710
>From 9aaa8cdd2cbad7a768abeb2cba4d81d049b9e049 Mon Sep 17 00:00:00 2001
From: tomershafir <tomer.shafir8 at gmail.com>
Date: Sun, 12 Apr 2026 14:13:26 +0300
Subject: [PATCH 1/2] [MacroFusion] Early return when insts already clustered
This patch adds an early return to `fuseInstructionPair()` when macro fused instructions are already clustered, either by an earlier fusion or another clustering like ld/st clustering, removing the assert.
The assert is generally wrong - there are edge cases where an earlier ld/st clustering (before macro fusion) reached the assert because it sets `ParentClusterIdx` and fails. For example, ADRP+LOAD/STORE on AArch64, thought it seems to be a rare case because the addresses are ususally unkown at compile time.
It doesn't effectively change how fusions are prioritized - early fusions still win on fusion-fusion conflicts, like before. But it changes how we resolve the edge case of ld/st-fusion conflicts:
Previously, fusions would effectively override ld/st clustering in this case, given that we currently limits instruction membership to at most a single cluster through `ParentClusterIdx`. Macro fusion runs after ld/st clustering in the pipelines.
Here we inverse the priorities from MacroFusion's perspective and prefer earlier ld/st clustering. I think they should be generally preferred over fusions, because they not only save dispatch and execution slots, but they also should decrease code size. Of course this heuristic can fail in some cases. If a new clustering algorithm is added, it would have to inspect ordering and be placed correctly.
I have inspected several internal workloads, and ld/st-fusion conflicts very rarely appear.
---
llvm/lib/CodeGen/MacroFusion.cpp | 28 +++----
.../AArch64/macro-fusion-cluster-conflict.mir | 75 +++++++++++++++++++
2 files changed, 87 insertions(+), 16 deletions(-)
create mode 100644 llvm/test/CodeGen/AArch64/macro-fusion-cluster-conflict.mir
diff --git a/llvm/lib/CodeGen/MacroFusion.cpp b/llvm/lib/CodeGen/MacroFusion.cpp
index 1db53017e6cef..55dadea676dbb 100644
--- a/llvm/lib/CodeGen/MacroFusion.cpp
+++ b/llvm/lib/CodeGen/MacroFusion.cpp
@@ -25,6 +25,9 @@
#define DEBUG_TYPE "machine-scheduler"
STATISTIC(NumFused, "Number of instr pairs fused");
+STATISTIC(NumFusionConflicts,
+ "Number of conflicts between a fusion pair and an already existing "
+ "cluster (either fusion or non-fusion)");
using namespace llvm;
@@ -52,22 +55,15 @@ bool llvm::hasLessThanNumFused(const SUnit &SU, unsigned FuseLimit) {
bool llvm::fuseInstructionPair(ScheduleDAGInstrs &DAG, SUnit &FirstSU,
SUnit &SecondSU) {
- // Check that neither instr is already paired with another along the edge
- // between them.
- for (SDep &SI : FirstSU.Succs)
- if (SI.isCluster())
- return false;
-
- for (SDep &SI : SecondSU.Preds)
- if (SI.isCluster())
- return false;
-
- assert(FirstSU.ParentClusterIdx == InvalidClusterId &&
- SecondSU.ParentClusterIdx == InvalidClusterId);
-
- // Though the reachability checks above could be made more generic,
- // perhaps as part of ScheduleDAGInstrs::addEdge(), since such edges are valid,
- // the extra computation cost makes it less interesting in general cases.
+ // Check that neither instr is already associated with a cluster (either
+ // fusion or non-fusion)
+ if (FirstSU.isClustered() || SecondSU.isClustered()) {
+ ++NumFusionConflicts;
+ LLVM_DEBUG(dbgs() << "Fusion conflict: cannot fuse SU(" << FirstSU.NodeNum
+ << ") and SU(" << SecondSU.NodeNum
+ << ") - already clustered\n");
+ return false;
+ }
// Create a single weak edge between the adjacent instrs. The only effect is
// to cause bottom-up scheduling to heavily prioritize the clustered instrs.
diff --git a/llvm/test/CodeGen/AArch64/macro-fusion-cluster-conflict.mir b/llvm/test/CodeGen/AArch64/macro-fusion-cluster-conflict.mir
new file mode 100644
index 0000000000000..5dd3a3c9c3e28
--- /dev/null
+++ b/llvm/test/CodeGen/AArch64/macro-fusion-cluster-conflict.mir
@@ -0,0 +1,75 @@
+# REQUIRES: aarch64-registered-target
+# REQUIRES: asserts
+
+# RUN: llc -o /dev/null %s -mtriple=aarch64 -mattr=+fuse-address \
+# RUN: -run-pass machine-scheduler -debug-only=machine-scheduler 2>&1 \
+# RUN: | FileCheck %s
+# RUN: llc -o /dev/null %s -mtriple=aarch64-apple-macos -mcpu=apple-m5 \
+# RUN: -run-pass machine-scheduler -debug-only=machine-scheduler 2>&1 \
+# RUN: | FileCheck %s
+
+# Test macro fusion conflicts with other macro fusions and with ld/st clustering.
+
+# Fusion-fusion conflict: ADRP fans out to two loads at non-adjacent offsets
+# (so they are NOT clustered by LoadClusterDAGMutation). Both ADRP→LDR pairs
+# are valid address fusion pairs. SU(0)→SU(1) fuses first; when SU(0)→SU(2)
+# is attempted, SU(0) is already clustered.
+---
+name: fusion-fusion-conflict
+tracksRegLiveness: true
+body: |
+ bb.0:
+ ; CHECK-LABEL: fusion-fusion-conflict
+ ; CHECK: Macro fuse: SU(0) - SU(1) / ADRP - LDRXui
+ ; CHECK: Fusion conflict: cannot fuse SU(0) and SU(2) - already clustered
+ ; CHECK: SU(0): $x0 = ADRP 0
+ ; CHECK: SU(1): $x1 = LDRXui $x0, 5
+ ; CHECK: SU(2): $x2 = LDRXui $x0, 10
+ $x0 = ADRP 0
+ $x1 = LDRXui $x0, 5 :: (load (s64))
+ $x2 = LDRXui $x0, 10 :: (load (s64))
+ RET_ReallyLR
+...
+
+# Store-fusion conflict: two adjacent stores get clustered by
+# StoreClusterDAGMutation (runs before AArch64 macro fusion). Then address
+# fusion (ADRP + STRXui) is attempted but the store SUnit is already
+# clustered.
+---
+name: store-fusion-conflict
+tracksRegLiveness: true
+body: |
+ bb.0:
+ liveins: $x0
+
+ ; CHECK-LABEL: store-fusion-conflict
+ ; CHECK: Cluster ld/st SU(1) - SU(2)
+ ; CHECK: Fusion conflict: cannot fuse SU(0) and SU(1) - already clustered
+ ; CHECK: SU(0): $x1 = ADRP 0
+ ; CHECK: SU(1): STRXui $x0, $x1, 0 :: (store (s64))
+ ; CHECK: SU(2): STRXui $x0, $x1, 1 :: (store (s64))
+ $x1 = ADRP 0
+ STRXui $x0, $x1, 0 :: (store (s64))
+ STRXui $x0, $x1, 1 :: (store (s64))
+ RET_ReallyLR
+...
+
+# Load-fusion conflict: two adjacent loads get clustered by
+# LoadClusterDAGMutation. Then address fusion (ADRP + LDRXui) is attempted
+# but the load SUnit is already clustered.
+---
+name: load-fusion-conflict
+tracksRegLiveness: true
+body: |
+ bb.0:
+ ; CHECK-LABEL: load-fusion-conflict
+ ; CHECK: Cluster ld/st SU(1) - SU(2)
+ ; CHECK: Fusion conflict: cannot fuse SU(0) and SU(1) - already clustered
+ ; CHECK: SU(0): $x0 = ADRP 0
+ ; CHECK: SU(1): $x1 = LDRXui $x0, 0
+ ; CHECK: SU(2): $x2 = LDRXui $x0, 1
+ $x0 = ADRP 0
+ $x1 = LDRXui $x0, 0 :: (load (s64))
+ $x2 = LDRXui $x0, 1 :: (load (s64))
+ RET_ReallyLR
+...
>From e5b7ea922ec8969524b40f8901305f320114c810 Mon Sep 17 00:00:00 2001
From: tomershafir <tomer.shafir8 at gmail.com>
Date: Mon, 13 Apr 2026 14:05:25 +0300
Subject: [PATCH 2/2] log which SU is clustered
---
llvm/lib/CodeGen/MacroFusion.cpp | 11 ++++++++---
.../CodeGen/AArch64/macro-fusion-cluster-conflict.mir | 9 ++++++---
2 files changed, 14 insertions(+), 6 deletions(-)
diff --git a/llvm/lib/CodeGen/MacroFusion.cpp b/llvm/lib/CodeGen/MacroFusion.cpp
index 55dadea676dbb..48448c8e82b58 100644
--- a/llvm/lib/CodeGen/MacroFusion.cpp
+++ b/llvm/lib/CodeGen/MacroFusion.cpp
@@ -59,9 +59,14 @@ bool llvm::fuseInstructionPair(ScheduleDAGInstrs &DAG, SUnit &FirstSU,
// fusion or non-fusion)
if (FirstSU.isClustered() || SecondSU.isClustered()) {
++NumFusionConflicts;
- LLVM_DEBUG(dbgs() << "Fusion conflict: cannot fuse SU(" << FirstSU.NodeNum
- << ") and SU(" << SecondSU.NodeNum
- << ") - already clustered\n");
+ LLVM_DEBUG({
+ dbgs() << "Fusion conflict: cannot fuse SU(" << FirstSU.NodeNum
+ << ") and SU(" << SecondSU.NodeNum << ")\n";
+ if (FirstSU.isClustered())
+ dbgs() << " SU(" << FirstSU.NodeNum << ") already clustered\n";
+ if (SecondSU.isClustered())
+ dbgs() << " SU(" << SecondSU.NodeNum << ") already clustered\n";
+ });
return false;
}
diff --git a/llvm/test/CodeGen/AArch64/macro-fusion-cluster-conflict.mir b/llvm/test/CodeGen/AArch64/macro-fusion-cluster-conflict.mir
index 5dd3a3c9c3e28..1aad0f0c6d310 100644
--- a/llvm/test/CodeGen/AArch64/macro-fusion-cluster-conflict.mir
+++ b/llvm/test/CodeGen/AArch64/macro-fusion-cluster-conflict.mir
@@ -21,7 +21,8 @@ body: |
bb.0:
; CHECK-LABEL: fusion-fusion-conflict
; CHECK: Macro fuse: SU(0) - SU(1) / ADRP - LDRXui
- ; CHECK: Fusion conflict: cannot fuse SU(0) and SU(2) - already clustered
+ ; CHECK: Fusion conflict: cannot fuse SU(0) and SU(2)
+ ; CHECK-NEXT: SU(0) already clustered
; CHECK: SU(0): $x0 = ADRP 0
; CHECK: SU(1): $x1 = LDRXui $x0, 5
; CHECK: SU(2): $x2 = LDRXui $x0, 10
@@ -44,7 +45,8 @@ body: |
; CHECK-LABEL: store-fusion-conflict
; CHECK: Cluster ld/st SU(1) - SU(2)
- ; CHECK: Fusion conflict: cannot fuse SU(0) and SU(1) - already clustered
+ ; CHECK: Fusion conflict: cannot fuse SU(0) and SU(1)
+ ; CHECK-NEXT: SU(1) already clustered
; CHECK: SU(0): $x1 = ADRP 0
; CHECK: SU(1): STRXui $x0, $x1, 0 :: (store (s64))
; CHECK: SU(2): STRXui $x0, $x1, 1 :: (store (s64))
@@ -64,7 +66,8 @@ body: |
bb.0:
; CHECK-LABEL: load-fusion-conflict
; CHECK: Cluster ld/st SU(1) - SU(2)
- ; CHECK: Fusion conflict: cannot fuse SU(0) and SU(1) - already clustered
+ ; CHECK: Fusion conflict: cannot fuse SU(0) and SU(1)
+ ; CHECK-NEXT: SU(1) already clustered
; CHECK: SU(0): $x0 = ADRP 0
; CHECK: SU(1): $x1 = LDRXui $x0, 0
; CHECK: SU(2): $x2 = LDRXui $x0, 1
More information about the llvm-commits
mailing list