[llvm] [RISCV] Handle ADD in RISCVInstrInfo::isCopyInstrImpl (PR #81123)

Alex Bradbury via llvm-commits llvm-commits at lists.llvm.org
Wed Mar 5 04:25:30 PST 2025


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

>From 05077e9b5ebb88090d62a7b21da8ce70facc4431 Mon Sep 17 00:00:00 2001
From: Alex Bradbury <asb at igalia.com>
Date: Thu, 8 Feb 2024 11:13:24 +0000
Subject: [PATCH 1/3] [RISCV] Handle ADD in RISCVInstrInfo::isCopyInstrImpl

Split out from #77610 and features a test, as a buggy version of this
caused a regression when landing that patch (the previous version had a
typo picking the wrong register as the source).
---
 llvm/lib/Target/RISCV/RISCVInstrInfo.cpp           |  6 ++++++
 llvm/unittests/Target/RISCV/RISCVInstrInfoTest.cpp | 10 +++++++---
 2 files changed, 13 insertions(+), 3 deletions(-)

diff --git a/llvm/lib/Target/RISCV/RISCVInstrInfo.cpp b/llvm/lib/Target/RISCV/RISCVInstrInfo.cpp
index 89eb71d917428..ce3f925cbc58a 100644
--- a/llvm/lib/Target/RISCV/RISCVInstrInfo.cpp
+++ b/llvm/lib/Target/RISCV/RISCVInstrInfo.cpp
@@ -1579,6 +1579,12 @@ RISCVInstrInfo::isCopyInstrImpl(const MachineInstr &MI) const {
   switch (MI.getOpcode()) {
   default:
     break;
+  case RISCV::ADD:
+    if (MI.getOperand(1).isReg() && MI.getOperand(1).getReg() == RISCV::X0)
+      return DestSourcePair{MI.getOperand(0), MI.getOperand(2)};
+    if (MI.getOperand(2).isReg() && MI.getOperand(2).getReg() == RISCV::X0)
+      return DestSourcePair{MI.getOperand(0), MI.getOperand(1)};
+    break;
   case RISCV::ADDI:
     // Operand 1 can be a frameindex but callers expect registers
     if (MI.getOperand(1).isReg() && MI.getOperand(2).isImm() &&
diff --git a/llvm/unittests/Target/RISCV/RISCVInstrInfoTest.cpp b/llvm/unittests/Target/RISCV/RISCVInstrInfoTest.cpp
index 5f3ce53f5d274..fab264c8e6b53 100644
--- a/llvm/unittests/Target/RISCV/RISCVInstrInfoTest.cpp
+++ b/llvm/unittests/Target/RISCV/RISCVInstrInfoTest.cpp
@@ -134,7 +134,7 @@ TEST_P(RISCVInstrInfoTest, IsCopyInstrImpl) {
   EXPECT_EQ(MI4Res->Destination->getReg(), RISCV::F1_D);
   EXPECT_EQ(MI4Res->Source->getReg(), RISCV::F2_D);
 
-  // ADD. TODO: Should return true for add reg, x0 and add x0, reg.
+  // ADD.
   MachineInstr *MI5 = BuildMI(*MF, DL, TII->get(RISCV::ADD), RISCV::X1)
                           .addReg(RISCV::X2)
                           .addReg(RISCV::X3)
@@ -147,14 +147,18 @@ TEST_P(RISCVInstrInfoTest, IsCopyInstrImpl) {
                           .addReg(RISCV::X2)
                           .getInstr();
   auto MI6Res = TII->isCopyInstrImpl(*MI6);
-  EXPECT_FALSE(MI6Res.has_value());
+  ASSERT_TRUE(MI6Res.has_value());
+  EXPECT_EQ(MI6Res->Destination->getReg(), RISCV::X1);
+  EXPECT_EQ(MI6Res->Source->getReg(), RISCV::X2);
 
   MachineInstr *MI7 = BuildMI(*MF, DL, TII->get(RISCV::ADD), RISCV::X1)
                           .addReg(RISCV::X2)
                           .addReg(RISCV::X0)
                           .getInstr();
   auto MI7Res = TII->isCopyInstrImpl(*MI7);
-  EXPECT_FALSE(MI7Res.has_value());
+  ASSERT_TRUE(MI7Res.has_value());
+  EXPECT_EQ(MI7Res->Destination->getReg(), RISCV::X1);
+  EXPECT_EQ(MI7Res->Source->getReg(), RISCV::X2);
 }
 
 TEST_P(RISCVInstrInfoTest, GetMemOperandsWithOffsetWidth) {

>From c1888295f152e02b506d2fd4cc658952579a012d Mon Sep 17 00:00:00 2001
From: Alex Bradbury <asb at igalia.com>
Date: Wed, 5 Mar 2025 12:20:47 +0000
Subject: [PATCH 2/3] As suggested in code review, add additional isReg checks

Not clear these are needed, but it is consistent.
---
 llvm/lib/Target/RISCV/RISCVInstrInfo.cpp | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/llvm/lib/Target/RISCV/RISCVInstrInfo.cpp b/llvm/lib/Target/RISCV/RISCVInstrInfo.cpp
index 535069a436e17..7d5f97bf964aa 100644
--- a/llvm/lib/Target/RISCV/RISCVInstrInfo.cpp
+++ b/llvm/lib/Target/RISCV/RISCVInstrInfo.cpp
@@ -1651,9 +1651,11 @@ RISCVInstrInfo::isCopyInstrImpl(const MachineInstr &MI) const {
   default:
     break;
   case RISCV::ADD:
-    if (MI.getOperand(1).isReg() && MI.getOperand(1).getReg() == RISCV::X0)
+    if (MI.getOperand(1).isReg() && MI.getOperand(1).getReg() == RISCV::X0 &&
+        MI.getOperand(2).isReg())
       return DestSourcePair{MI.getOperand(0), MI.getOperand(2)};
-    if (MI.getOperand(2).isReg() && MI.getOperand(2).getReg() == RISCV::X0)
+    if (MI.getOperand(2).isReg() && MI.getOperand(2).getReg() == RISCV::X0 &&
+        MI.getOperand(1).isReg())
       return DestSourcePair{MI.getOperand(0), MI.getOperand(1)};
     break;
   case RISCV::ADDI:

>From 3f40a2cd9cd985a3cf7fda24fedaf2216c5e2118 Mon Sep 17 00:00:00 2001
From: Alex Bradbury <asb at igalia.com>
Date: Wed, 5 Mar 2025 12:24:32 +0000
Subject: [PATCH 3/3] Update constbarrier-rv32.ll based on this change

---
 llvm/test/CodeGen/RISCV/GlobalISel/constbarrier-rv32.ll | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/llvm/test/CodeGen/RISCV/GlobalISel/constbarrier-rv32.ll b/llvm/test/CodeGen/RISCV/GlobalISel/constbarrier-rv32.ll
index cf7cef83bcc13..2fcb911c2654a 100644
--- a/llvm/test/CodeGen/RISCV/GlobalISel/constbarrier-rv32.ll
+++ b/llvm/test/CodeGen/RISCV/GlobalISel/constbarrier-rv32.ll
@@ -33,13 +33,13 @@ define void @constant_fold_barrier_i128(ptr %p) {
 ; RV32-NEXT:    add a2, a2, a1
 ; RV32-NEXT:    add a6, a3, zero
 ; RV32-NEXT:    sltu a1, a2, a1
-; RV32-NEXT:    sltu a3, a6, a3
+; RV32-NEXT:    sltu a3, a3, a3
 ; RV32-NEXT:    add a6, a6, a1
 ; RV32-NEXT:    seqz a7, a6
 ; RV32-NEXT:    and a1, a7, a1
 ; RV32-NEXT:    add a7, a4, zero
 ; RV32-NEXT:    add a5, a5, zero
-; RV32-NEXT:    sltu a4, a7, a4
+; RV32-NEXT:    sltu a4, a4, a4
 ; RV32-NEXT:    or a1, a3, a1
 ; RV32-NEXT:    add a7, a7, a1
 ; RV32-NEXT:    seqz a3, a7



More information about the llvm-commits mailing list