[llvm] [SelectionDAG] Combine range metadata when loads are CSEd. (PR #146026)
Craig Topper via llvm-commits
llvm-commits at lists.llvm.org
Thu Jun 26 22:47:22 PDT 2025
https://github.com/topperc updated https://github.com/llvm/llvm-project/pull/146026
>From 81eb3be4cc0a3657adf23e9817a2872751cd70e7 Mon Sep 17 00:00:00 2001
From: Craig Topper <craig.topper at sifive.com>
Date: Thu, 26 Jun 2025 21:27:33 -0700
Subject: [PATCH 1/2] Pre-commit test
---
llvm/test/CodeGen/RISCV/pr145363.ll | 40 +++++++++++++++++++++++++++++
1 file changed, 40 insertions(+)
create mode 100644 llvm/test/CodeGen/RISCV/pr145363.ll
diff --git a/llvm/test/CodeGen/RISCV/pr145363.ll b/llvm/test/CodeGen/RISCV/pr145363.ll
new file mode 100644
index 0000000000000..9a3fcd466f575
--- /dev/null
+++ b/llvm/test/CodeGen/RISCV/pr145363.ll
@@ -0,0 +1,40 @@
+; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py UTC_ARGS: --version 5
+; RUN: llc < %s -mtriple=riscv64 | FileCheck %s
+
+; These two loads will CSE, we need to conversatively combine the range
+; metadata. The final assembly should not contain an OR.
+define i32 @f(ptr %p) {
+; CHECK-LABEL: f:
+; CHECK: # %bb.0:
+; CHECK-NEXT: lw a0, 0(a0)
+; CHECK-NEXT: lui a1, 294471
+; CHECK-NEXT: addi a1, a1, 1064
+; CHECK-NEXT: or a0, a0, a1
+; CHECK-NEXT: ret
+ %load = load i32, ptr %p, align 4, !range !0
+ %load2 = load i32, ptr %p, align 4
+ %add = add i32 1206154280, %load2
+ ret i32 %add
+}
+
+; The mul and getelementptr will get removed in DAGCombine causing the loads
+; to CSE after they are created.
+define i32 @test(ptr %p, i32 %x, ptr %q) {
+; CHECK-LABEL: test:
+; CHECK: # %bb.0:
+; CHECK-NEXT: lw a1, 0(a0)
+; CHECK-NEXT: lui a0, 294471
+; CHECK-NEXT: addi a0, a0, 1064
+; CHECK-NEXT: or a0, a1, a0
+; CHECK-NEXT: sw a1, 0(a2)
+; CHECK-NEXT: ret
+ %load = load i32, ptr %p, align 4, !range !0
+ %mul = mul i32 0, %x
+ %a = getelementptr i32, ptr %p, i32 %mul
+ %load2 = load i32, ptr %a, align 4
+ %add = add i32 1206154280, %load2
+ store i32 %load, ptr %q
+ ret i32 %add
+}
+
+!0 = !{i32 1, i32 2, i32 3, i32 4}
>From eeb9aaa0f13d030a43328378a94067e4c119ba5a Mon Sep 17 00:00:00 2001
From: Craig Topper <craig.topper at sifive.com>
Date: Thu, 26 Jun 2025 21:29:56 -0700
Subject: [PATCH 2/2] [SelectionDAG] Combine range metadata when loads are
CSEd.
When CSEing a load with an existing load with different range
metadata, clear the range metadata on the existing
load.
This is conservative, alternatively we could calculate new range
metadata using getMostGenericRange. Without a test case I wasn't
sure it was worth it.
getMostGenericRange takes a non-const MDNode*, but all of SelectionDAG
uses const MDNode*. A const_cast will need to be used somewhere or
we need to make the code base consistent about whether MDNode pointers
should be const or not.
I'm sure this isn't the only place that needs to be updated to handle
the CSE.
Fixes #145363.
---
llvm/include/llvm/CodeGen/SelectionDAGNodes.h | 8 ++++++++
llvm/lib/CodeGen/SelectionDAG/SelectionDAG.cpp | 17 +++++++++++------
llvm/test/CodeGen/RISCV/pr145363.ll | 4 ++--
3 files changed, 21 insertions(+), 8 deletions(-)
diff --git a/llvm/include/llvm/CodeGen/SelectionDAGNodes.h b/llvm/include/llvm/CodeGen/SelectionDAGNodes.h
index 92da4ef7f0556..a3675eecfea3f 100644
--- a/llvm/include/llvm/CodeGen/SelectionDAGNodes.h
+++ b/llvm/include/llvm/CodeGen/SelectionDAGNodes.h
@@ -1497,6 +1497,14 @@ class MemSDNode : public SDNode {
MMO->refineAlignment(NewMMO);
}
+ void refineRanges(const MachineMemOperand *NewMMO) {
+ // If this node has range metadata that is different than NewMMO, clear the
+ // range metadata.
+ // FIXME: Union the ranges instead?
+ if (getRanges() && getRanges() != NewMMO->getRanges())
+ MMO->clearRanges();
+ }
+
const SDValue &getChain() const { return getOperand(0); }
const SDValue &getBasePtr() const {
diff --git a/llvm/lib/CodeGen/SelectionDAG/SelectionDAG.cpp b/llvm/lib/CodeGen/SelectionDAG/SelectionDAG.cpp
index 30ee6a99b9dfc..5fc1e7f4d1cd1 100644
--- a/llvm/lib/CodeGen/SelectionDAG/SelectionDAG.cpp
+++ b/llvm/lib/CodeGen/SelectionDAG/SelectionDAG.cpp
@@ -1275,6 +1275,8 @@ SelectionDAG::AddModifiedNodeToCSEMaps(SDNode *N) {
// to replace the dead one with the existing one. This can cause
// recursive merging of other unrelated nodes down the line.
Existing->intersectFlagsWith(N->getFlags());
+ if (auto *MemNode = dyn_cast<MemSDNode>(Existing))
+ MemNode->refineRanges(cast<MemSDNode>(N)->getMemOperand());
ReplaceAllUsesWith(N, Existing);
// N is now dead. Inform the listeners and delete it.
@@ -9109,8 +9111,9 @@ SDValue SelectionDAG::getAtomic(unsigned Opcode, const SDLoc &dl, EVT MemVT,
ID.AddInteger(MMO->getPointerInfo().getAddrSpace());
ID.AddInteger(MMO->getFlags());
void* IP = nullptr;
- if (SDNode *E = FindNodeOrInsertPos(ID, dl, IP)) {
- cast<AtomicSDNode>(E)->refineAlignment(MMO);
+ if (auto *E = cast_or_null<AtomicSDNode>(FindNodeOrInsertPos(ID, dl, IP))) {
+ E->refineAlignment(MMO);
+ E->refineRanges(MMO);
return SDValue(E, 0);
}
@@ -9401,8 +9404,9 @@ SDValue SelectionDAG::getLoad(ISD::MemIndexedMode AM, ISD::LoadExtType ExtType,
ID.AddInteger(MMO->getPointerInfo().getAddrSpace());
ID.AddInteger(MMO->getFlags());
void *IP = nullptr;
- if (SDNode *E = FindNodeOrInsertPos(ID, dl, IP)) {
- cast<LoadSDNode>(E)->refineAlignment(MMO);
+ if (auto *E = cast_or_null<LoadSDNode>(FindNodeOrInsertPos(ID, dl, IP))) {
+ E->refineAlignment(MMO);
+ E->refineRanges(MMO);
return SDValue(E, 0);
}
auto *N = newSDNode<LoadSDNode>(dl.getIROrder(), dl.getDebugLoc(), VTs, AM,
@@ -9622,8 +9626,9 @@ SDValue SelectionDAG::getLoadVP(ISD::MemIndexedMode AM,
ID.AddInteger(MMO->getPointerInfo().getAddrSpace());
ID.AddInteger(MMO->getFlags());
void *IP = nullptr;
- if (SDNode *E = FindNodeOrInsertPos(ID, dl, IP)) {
- cast<VPLoadSDNode>(E)->refineAlignment(MMO);
+ if (auto *E = cast_or_null<VPLoadSDNode>(FindNodeOrInsertPos(ID, dl, IP))) {
+ E->refineAlignment(MMO);
+ E->refineRanges(MMO);
return SDValue(E, 0);
}
auto *N = newSDNode<VPLoadSDNode>(dl.getIROrder(), dl.getDebugLoc(), VTs, AM,
diff --git a/llvm/test/CodeGen/RISCV/pr145363.ll b/llvm/test/CodeGen/RISCV/pr145363.ll
index 9a3fcd466f575..1270f1d5dbda0 100644
--- a/llvm/test/CodeGen/RISCV/pr145363.ll
+++ b/llvm/test/CodeGen/RISCV/pr145363.ll
@@ -9,7 +9,7 @@ define i32 @f(ptr %p) {
; CHECK-NEXT: lw a0, 0(a0)
; CHECK-NEXT: lui a1, 294471
; CHECK-NEXT: addi a1, a1, 1064
-; CHECK-NEXT: or a0, a0, a1
+; CHECK-NEXT: addw a0, a0, a1
; CHECK-NEXT: ret
%load = load i32, ptr %p, align 4, !range !0
%load2 = load i32, ptr %p, align 4
@@ -25,7 +25,7 @@ define i32 @test(ptr %p, i32 %x, ptr %q) {
; CHECK-NEXT: lw a1, 0(a0)
; CHECK-NEXT: lui a0, 294471
; CHECK-NEXT: addi a0, a0, 1064
-; CHECK-NEXT: or a0, a1, a0
+; CHECK-NEXT: addw a0, a1, a0
; CHECK-NEXT: sw a1, 0(a2)
; CHECK-NEXT: ret
%load = load i32, ptr %p, align 4, !range !0
More information about the llvm-commits
mailing list