[llvm] [RISCV] Change heuristic used for load clustering (PR #75341)

Alex Bradbury via llvm-commits llvm-commits at lists.llvm.org
Tue Jan 2 08:27:19 PST 2024


https://github.com/asb updated https://github.com/llvm/llvm-project/pull/75341

>From 5785636fdf5170b27e3157515bafb4565fa5aa2a Mon Sep 17 00:00:00 2001
From: Alex Bradbury <asb at igalia.com>
Date: Wed, 13 Dec 2023 13:50:55 +0000
Subject: [PATCH 1/2] [RISCV] Change heuristic used for load clustering

Split out from #73789. Clusters if the operations are within a cache
line of each other (as AMDGPU does in shouldScheduleLoadsNear). X86 does
something similar, but does `((Offset2 - Offset1) / 8 > 64)`. I'm not
sure if that's intentionally set to 512 bytes or if the division is in
error.

Adopts the suggestion from @wangpc-pp to query the cache line size and
use it if available.
---
 llvm/lib/Target/RISCV/RISCVInstrInfo.cpp | 10 +++++++---
 1 file changed, 7 insertions(+), 3 deletions(-)

diff --git a/llvm/lib/Target/RISCV/RISCVInstrInfo.cpp b/llvm/lib/Target/RISCV/RISCVInstrInfo.cpp
index 1dcff7eb563e20..b8960fd9571c9b 100644
--- a/llvm/lib/Target/RISCV/RISCVInstrInfo.cpp
+++ b/llvm/lib/Target/RISCV/RISCVInstrInfo.cpp
@@ -2282,9 +2282,13 @@ bool RISCVInstrInfo::shouldClusterMemOps(
     return false;
   }
 
-  // TODO: Use a more carefully chosen heuristic, e.g. only cluster if offsets
-  // indicate they likely share a cache line.
-  return ClusterSize <= 4;
+  unsigned CacheLineSize =
+      BaseOps1.front()->getParent()->getMF()->getSubtarget().getCacheLineSize();
+  // Assume a cache line size of 64 bytes if no size is set in RISCVSubtarget.
+  CacheLineSize = CacheLineSize ? CacheLineSize : 64;
+  // Cluster if the memory operations are on the same or a neighbouring cache
+  // line.
+  return std::abs(Offset1 - Offset2) < CacheLineSize;
 }
 
 // Set BaseReg (the base register operand), Offset (the byte offset being

>From 96e8828704c1c964cc9aceac6e7d8c34f7c04264 Mon Sep 17 00:00:00 2001
From: Alex Bradbury <asb at igalia.com>
Date: Wed, 13 Dec 2023 19:34:46 +0000
Subject: [PATCH 2/2] Tweak heuristic to limit the maximum cluster size

---
 llvm/lib/Target/RISCV/RISCVInstrInfo.cpp | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/llvm/lib/Target/RISCV/RISCVInstrInfo.cpp b/llvm/lib/Target/RISCV/RISCVInstrInfo.cpp
index b8960fd9571c9b..cd98438eed8821 100644
--- a/llvm/lib/Target/RISCV/RISCVInstrInfo.cpp
+++ b/llvm/lib/Target/RISCV/RISCVInstrInfo.cpp
@@ -2287,8 +2287,9 @@ bool RISCVInstrInfo::shouldClusterMemOps(
   // Assume a cache line size of 64 bytes if no size is set in RISCVSubtarget.
   CacheLineSize = CacheLineSize ? CacheLineSize : 64;
   // Cluster if the memory operations are on the same or a neighbouring cache
-  // line.
-  return std::abs(Offset1 - Offset2) < CacheLineSize;
+  // line, but limit the maximum ClusterSize to avoid creating too much
+  // additional register pressure.
+  return ClusterSize <= 4 && std::abs(Offset1 - Offset2) < CacheLineSize;
 }
 
 // Set BaseReg (the base register operand), Offset (the byte offset being



More information about the llvm-commits mailing list