[llvm] [RISCV] Add OR/XOR/SUB to RISCVInstrInfo::isCopyInstrImpl (PR #132002)

[via llvm-commits]

llvmbot wrote:


<!--LLVM PR SUMMARY COMMENT-->

@llvm/pr-subscribers-backend-risc-v

Author: Alex Bradbury (asb)

<details>
<summary>Changes</summary>

This adds coverage for additional instructions in isCopyInstrImpl, for now picking just those where I can observe that there is a codegen difference for SPEC.

---

Implementing these removes a small number of effectively no-op instructions in a SPEC compilation. As a starting point I've just added the cases that cause a code gen difference, but welcome discussion on to what degree we want isCopyInstrImpl to be exhaustive.

I would highlight:
* The case of OR with indentical register operands isn't covered. Although this causes no codegen difference, possibly I should update this PR to handle it? That way at least the opcodes that isCopyInstrImpl recognises are covered exhaustively.
* Other instructions tried that have no impact on SPEC codegen:
  * ORI/XORI with imm == 0
  * ANDN with second operand == X0
  * ROL/ROR with X0 rotate value
  * MIN/MINU/MAX/MAXU with equal operands


---
Full diff: https://github.com/llvm/llvm-project/pull/132002.diff


2 Files Affected:

- (modified) llvm/lib/Target/RISCV/RISCVInstrInfo.cpp (+7) 
- (modified) llvm/unittests/Target/RISCV/RISCVInstrInfoTest.cpp (+36-17) 


``````````diff
diff --git a/llvm/lib/Target/RISCV/RISCVInstrInfo.cpp b/llvm/lib/Target/RISCV/RISCVInstrInfo.cpp
index c197f06855b6e..84e167e452130 100644
--- a/llvm/lib/Target/RISCV/RISCVInstrInfo.cpp
+++ b/llvm/lib/Target/RISCV/RISCVInstrInfo.cpp
@@ -1660,6 +1660,8 @@ RISCVInstrInfo::isCopyInstrImpl(const MachineInstr &MI) const {
   default:
     break;
   case RISCV::ADD:
+  case RISCV::OR:
+  case RISCV::XOR:
     if (MI.getOperand(1).isReg() && MI.getOperand(1).getReg() == RISCV::X0 &&
         MI.getOperand(2).isReg())
       return DestSourcePair{MI.getOperand(0), MI.getOperand(2)};
@@ -1673,6 +1675,11 @@ RISCVInstrInfo::isCopyInstrImpl(const MachineInstr &MI) const {
         MI.getOperand(2).getImm() == 0)
       return DestSourcePair{MI.getOperand(0), MI.getOperand(1)};
     break;
+  case RISCV::SUB:
+    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::FSGNJ_D:
   case RISCV::FSGNJ_S:
   case RISCV::FSGNJ_H:
diff --git a/llvm/unittests/Target/RISCV/RISCVInstrInfoTest.cpp b/llvm/unittests/Target/RISCV/RISCVInstrInfoTest.cpp
index f7351f00ae282..ac802c8d33fa1 100644
--- a/llvm/unittests/Target/RISCV/RISCVInstrInfoTest.cpp
+++ b/llvm/unittests/Target/RISCV/RISCVInstrInfoTest.cpp
@@ -135,31 +135,50 @@ TEST_P(RISCVInstrInfoTest, IsCopyInstrImpl) {
   EXPECT_EQ(MI4Res->Destination->getReg(), RISCV::F1_D);
   EXPECT_EQ(MI4Res->Source->getReg(), RISCV::F2_D);
 
-  // ADD.
-  MachineInstr *MI5 = BuildMI(*MF, DL, TII->get(RISCV::ADD), RISCV::X1)
-                          .addReg(RISCV::X2)
-                          .addReg(RISCV::X3)
-                          .getInstr();
-  auto MI5Res = TII->isCopyInstrImpl(*MI5);
-  EXPECT_FALSE(MI5Res.has_value());
+  // ADD/OR/XOR.
+  for (unsigned Opc : {RISCV::ADD, RISCV::OR, RISCV::XOR}) {
+    MachineInstr *MI5 = BuildMI(*MF, DL, TII->get(Opc), RISCV::X1)
+                            .addReg(RISCV::X2)
+                            .addReg(RISCV::X3)
+                            .getInstr();
+    auto MI5Res = TII->isCopyInstrImpl(*MI5);
+    EXPECT_FALSE(MI5Res.has_value());
+
+    MachineInstr *MI6 = BuildMI(*MF, DL, TII->get(Opc), RISCV::X1)
+                            .addReg(RISCV::X0)
+                            .addReg(RISCV::X2)
+                            .getInstr();
+    auto MI6Res = TII->isCopyInstrImpl(*MI6);
+    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(Opc), RISCV::X1)
+                            .addReg(RISCV::X2)
+                            .addReg(RISCV::X0)
+                            .getInstr();
+    auto MI7Res = TII->isCopyInstrImpl(*MI7);
+    ASSERT_TRUE(MI7Res.has_value());
+    EXPECT_EQ(MI7Res->Destination->getReg(), RISCV::X1);
+    EXPECT_EQ(MI7Res->Source->getReg(), RISCV::X2);
+  }
 
-  MachineInstr *MI6 = BuildMI(*MF, DL, TII->get(RISCV::ADD), RISCV::X1)
+  // SUB.
+  MachineInstr *MI8 = BuildMI(*MF, DL, TII->get(RISCV::SUB), RISCV::X1)
                           .addReg(RISCV::X0)
                           .addReg(RISCV::X2)
                           .getInstr();
-  auto MI6Res = TII->isCopyInstrImpl(*MI6);
-  ASSERT_TRUE(MI6Res.has_value());
-  EXPECT_EQ(MI6Res->Destination->getReg(), RISCV::X1);
-  EXPECT_EQ(MI6Res->Source->getReg(), RISCV::X2);
+  auto MI8Res = TII->isCopyInstrImpl(*MI8);
+  EXPECT_FALSE(MI8Res.has_value());
 
-  MachineInstr *MI7 = BuildMI(*MF, DL, TII->get(RISCV::ADD), RISCV::X1)
+  MachineInstr *MI9 = BuildMI(*MF, DL, TII->get(RISCV::SUB), RISCV::X1)
                           .addReg(RISCV::X2)
                           .addReg(RISCV::X0)
                           .getInstr();
-  auto MI7Res = TII->isCopyInstrImpl(*MI7);
-  ASSERT_TRUE(MI7Res.has_value());
-  EXPECT_EQ(MI7Res->Destination->getReg(), RISCV::X1);
-  EXPECT_EQ(MI7Res->Source->getReg(), RISCV::X2);
+  auto MI9Res = TII->isCopyInstrImpl(*MI9);
+  ASSERT_TRUE(MI9Res.has_value());
+  EXPECT_EQ(MI9Res->Destination->getReg(), RISCV::X1);
+  EXPECT_EQ(MI9Res->Source->getReg(), RISCV::X2);
 }
 
 TEST_P(RISCVInstrInfoTest, GetMemOperandsWithOffsetWidth) {

``````````

</details>


https://github.com/llvm/llvm-project/pull/132002