[llvm] 72a3cd9 - [MacroFusion] Early return when insts already clustered (#191710)
via llvm-commits
llvm-commits at lists.llvm.org
Wed Apr 15 22:51:01 PDT 2026
Author: Tomer Shafir
Date: 2026-04-16T08:50:56+03:00
New Revision: 72a3cd9e54f16d87f47bc984672999a9b67cd4f4
URL: https://github.com/llvm/llvm-project/commit/72a3cd9e54f16d87f47bc984672999a9b67cd4f4
DIFF: https://github.com/llvm/llvm-project/commit/72a3cd9e54f16d87f47bc984672999a9b67cd4f4.diff
LOG: [MacroFusion] Early return when insts already clustered (#191710)
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.
Added:
llvm/test/CodeGen/AArch64/macro-fusion-cluster-conflict.mir
Modified:
llvm/lib/CodeGen/MacroFusion.cpp
Removed:
################################################################################
diff --git a/llvm/lib/CodeGen/MacroFusion.cpp b/llvm/lib/CodeGen/MacroFusion.cpp
index 1db53017e6cef..48448c8e82b58 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,20 @@ 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 << ")\n";
+ if (FirstSU.isClustered())
+ dbgs() << " SU(" << FirstSU.NodeNum << ") already clustered\n";
+ if (SecondSU.isClustered())
+ dbgs() << " 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..87e2c00610ed2
--- /dev/null
+++ b/llvm/test/CodeGen/AArch64/macro-fusion-cluster-conflict.mir
@@ -0,0 +1,162 @@
+# 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.
+#
+# Conflict matrix (conflict-source × which-SU-is-clustered):
+#
+# | first-clustered | second-clustered | both-clustered
+# -----------------+-----------------+------------------+----------------
+# fusion-fusion | TEST | (a) | (a)
+# load-fusion | (b) | TEST | TEST
+# store-fusion | (b) | TEST | TEST
+#
+# Unconstructible cases:
+# (a) fusion-fusion second/both-clustered: the anchor (SecondSU) is visited
+# exactly once in the fusion loop. It can only be pre-clustered by ld/st
+# clustering (a
diff erent mutation), never by a prior fusion iteration.
+# both-clustered requires SecondSU to be fusion-clustered, same issue.
+# (b) load/store-fusion first-clustered: ADRP is not a load/store, so it
+# cannot be ld/st clustered. The only way ADRP gets clustered is via a
+# prior fusion, making it identical to fusion-fusion first-clustered.
+# There are currently no other fusions with loads/stores on AArch64.
+#------------------------------------------------------------------------------
+
+#------------------------------------------------------------------------------
+# Fusion-fusion conflicts
+#------------------------------------------------------------------------------
+
+# Fusion-fusion, first-clustered: 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-first-clustered
+tracksRegLiveness: true
+body: |
+ bb.0:
+ ; CHECK-LABEL: fusion-fusion-first-clustered
+ ; CHECK: Macro fuse: SU(0) - SU(1) / ADRP - LDRXui
+ ; 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 :: (load (s64))
+ ; CHECK: SU(2): $x2 = LDRXui $x0, 10 :: (load (s64))
+ $x0 = ADRP 0
+ $x1 = LDRXui $x0, 5 :: (load (s64))
+ $x2 = LDRXui $x0, 10 :: (load (s64))
+ RET_ReallyLR
+...
+
+#------------------------------------------------------------------------------
+# Load-fusion conflicts
+#------------------------------------------------------------------------------
+
+# Load-fusion, second-clustered: two adjacent loads get clustered by
+# LoadClusterDAGMutation. Then address fusion (ADRP + LDRXui) is attempted
+# but the load SUnit is already clustered.
+---
+name: load-fusion-second-clustered
+tracksRegLiveness: true
+body: |
+ bb.0:
+ ; CHECK-LABEL: load-fusion-second-clustered
+ ; CHECK: Cluster ld/st SU(1) - SU(2)
+ ; 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 :: (load (s64))
+ ; CHECK: SU(2): $x2 = LDRXui $x0, 1 :: (load (s64))
+ $x0 = ADRP 0
+ $x1 = LDRXui $x0, 0 :: (load (s64))
+ $x2 = LDRXui $x0, 1 :: (load (s64))
+ RET_ReallyLR
+...
+
+# Load-fusion, both-clustered: LDR2 and LDR3 get ld/st clustered (adjacent
+# offsets), then ADRP fuses with LDR1 (non-adjacent offset). When fusion tries
+# ADRP + LDR2, both are already clustered.
+---
+name: load-fusion-both-clustered
+tracksRegLiveness: true
+body: |
+ bb.0:
+ ; CHECK-LABEL: load-fusion-both-clustered
+ ; CHECK: Cluster ld/st SU(2) - SU(3)
+ ; CHECK: Macro fuse: SU(0) - SU(1) / ADRP - LDRXui
+ ; CHECK: Fusion conflict: cannot fuse SU(0) and SU(2)
+ ; CHECK-NEXT: SU(0) already clustered
+ ; CHECK-NEXT: SU(2) already clustered
+ ; CHECK: SU(0): $x0 = ADRP 0
+ ; CHECK: SU(1): $x1 = LDRXui $x0, 5 :: (load (s64))
+ ; CHECK: SU(2): $x2 = LDRXui $x0, 0 :: (load (s64))
+ ; CHECK: SU(3): $x3 = LDRXui $x0, 1 :: (load (s64))
+ $x0 = ADRP 0
+ $x1 = LDRXui $x0, 5 :: (load (s64))
+ $x2 = LDRXui $x0, 0 :: (load (s64))
+ $x3 = LDRXui $x0, 1 :: (load (s64))
+ RET_ReallyLR
+...
+
+#------------------------------------------------------------------------------
+# Store-fusion conflicts
+#------------------------------------------------------------------------------
+
+# Store-fusion, second-clustered: 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-second-clustered
+tracksRegLiveness: true
+body: |
+ bb.0:
+ liveins: $x0
+
+ ; CHECK-LABEL: store-fusion-second-clustered
+ ; CHECK: Cluster ld/st SU(1) - SU(2)
+ ; 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))
+ $x1 = ADRP 0
+ STRXui $x0, $x1, 0 :: (store (s64))
+ STRXui $x0, $x1, 1 :: (store (s64))
+ RET_ReallyLR
+...
+
+# Store-fusion, both-clustered: STR2 and STR3 get ld/st clustered (adjacent
+# offsets), then ADRP fuses with STR1 (non-adjacent offset). When fusion tries
+# ADRP + STR2, both are already clustered.
+---
+name: store-fusion-both-clustered
+tracksRegLiveness: true
+body: |
+ bb.0:
+ liveins: $x0
+
+ ; CHECK-LABEL: store-fusion-both-clustered
+ ; CHECK: Cluster ld/st SU(2) - SU(3)
+ ; CHECK: Macro fuse: SU(0) - SU(1) / ADRP - STRXui
+ ; CHECK: Fusion conflict: cannot fuse SU(0) and SU(2)
+ ; CHECK-NEXT: SU(0) already clustered
+ ; CHECK-NEXT: SU(2) already clustered
+ ; CHECK: SU(0): $x1 = ADRP 0
+ ; CHECK: SU(1): STRXui $x0, $x1, 5 :: (store (s64))
+ ; CHECK: SU(2): STRXui $x0, $x1, 0 :: (store (s64))
+ ; CHECK: SU(3): STRXui $x0, $x1, 1 :: (store (s64))
+ $x1 = ADRP 0
+ STRXui $x0, $x1, 5 :: (store (s64))
+ STRXui $x0, $x1, 0 :: (store (s64))
+ STRXui $x0, $x1, 1 :: (store (s64))
+ RET_ReallyLR
+...
More information about the llvm-commits
mailing list