From 5785636fdf5170b27e3157515bafb4565fa5aa2a Mon Sep 17 00:00:00 2001 From: Alex Bradbury 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 1dcff7eb563e2..b8960fd9571c9 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 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 b8960fd9571c9..cd98438eed882 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