[llvm] Reland [RISCV] Implement RISCVInsrInfo::getConstValDefinedInReg (PR #81124)
Alex Bradbury via llvm-commits
llvm-commits at lists.llvm.org
Thu Feb 8 03:22:16 PST 2024
https://github.com/asb created https://github.com/llvm/llvm-project/pull/81124
Reland of #77610, with the previously buggy isCopyInstrImpl change split out to #81123.
This helper function handles common cases where we can determine a
constant value is being defined in a register. Although it looks like
codegen changes are possible due to this being called in
PeepholeOptimizer, my main motivation is to use this in
describeLoadedValue.
>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/2] [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 89eb71d917428e..ce3f925cbc58a7 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 5f3ce53f5d274e..fab264c8e6b53c 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 20f59583b7deeb39efbceb64d6b62f4bb36cf901 Mon Sep 17 00:00:00 2001
From: Alex Bradbury <asb at igalia.com>
Date: Thu, 8 Feb 2024 11:19:25 +0000
Subject: [PATCH 2/2] Reland [RISCV] Implement
RISCVInsrInfo::getConstValDefinedInReg
Reland of #77610, with the previously buggy isCopyInstrImpl change split
out to #81123.
This helper function handles common cases where we can determine a
constant value is being defined in a register. Although it looks like
codegen changes are possible due to this being called in
PeepholeOptimizer, my main motivation is to use this in
describeLoadedValue.
---
llvm/lib/Target/RISCV/RISCVInstrInfo.cpp | 27 +++++++++++
llvm/lib/Target/RISCV/RISCVInstrInfo.h | 3 ++
.../Target/RISCV/RISCVInstrInfoTest.cpp | 46 +++++++++++++++++++
3 files changed, 76 insertions(+)
diff --git a/llvm/lib/Target/RISCV/RISCVInstrInfo.cpp b/llvm/lib/Target/RISCV/RISCVInstrInfo.cpp
index ce3f925cbc58a7..4390f587a94154 100644
--- a/llvm/lib/Target/RISCV/RISCVInstrInfo.cpp
+++ b/llvm/lib/Target/RISCV/RISCVInstrInfo.cpp
@@ -2562,6 +2562,33 @@ std::optional<RegImmPair> RISCVInstrInfo::isAddImmediate(const MachineInstr &MI,
return std::nullopt;
}
+bool RISCVInstrInfo::getConstValDefinedInReg(const MachineInstr &MI,
+ const Register Reg,
+ int64_t &ImmVal) const {
+ // Handle moves of X0.
+ if (auto DestSrc = isCopyInstr(MI)) {
+ if (DestSrc->Source->getReg() != RISCV::X0)
+ return false;
+ const Register DstReg = DestSrc->Destination->getReg();
+ if (DstReg != Reg)
+ return false;
+ ImmVal = 0;
+ return true;
+ }
+
+ if (!(MI.getOpcode() == RISCV::ADDI || MI.getOpcode() == RISCV::ADDIW ||
+ MI.getOpcode() == RISCV::ORI))
+ return false;
+ if (MI.getOperand(0).getReg() != Reg)
+ return false;
+ if (!MI.getOperand(1).isReg() || MI.getOperand(1).getReg() != RISCV::X0)
+ return false;
+ if (!MI.getOperand(2).isImm())
+ return false;
+ ImmVal = MI.getOperand(2).getImm();
+ return true;
+}
+
// MIR printer helper function to annotate Operands with a comment.
std::string RISCVInstrInfo::createMIROperandComment(
const MachineInstr &MI, const MachineOperand &Op, unsigned OpIdx,
diff --git a/llvm/lib/Target/RISCV/RISCVInstrInfo.h b/llvm/lib/Target/RISCV/RISCVInstrInfo.h
index 0f7d3e4e433908..5d20276696aea5 100644
--- a/llvm/lib/Target/RISCV/RISCVInstrInfo.h
+++ b/llvm/lib/Target/RISCV/RISCVInstrInfo.h
@@ -212,6 +212,9 @@ class RISCVInstrInfo : public RISCVGenInstrInfo {
std::optional<RegImmPair> isAddImmediate(const MachineInstr &MI,
Register Reg) const override;
+ bool getConstValDefinedInReg(const MachineInstr &MI, const Register Reg,
+ int64_t &ImmVal) const override;
+
bool findCommutedOpIndices(const MachineInstr &MI, unsigned &SrcOpIdx1,
unsigned &SrcOpIdx2) const override;
MachineInstr *commuteInstructionImpl(MachineInstr &MI, bool NewMI,
diff --git a/llvm/unittests/Target/RISCV/RISCVInstrInfoTest.cpp b/llvm/unittests/Target/RISCV/RISCVInstrInfoTest.cpp
index fab264c8e6b53c..ae3cb78538fefd 100644
--- a/llvm/unittests/Target/RISCV/RISCVInstrInfoTest.cpp
+++ b/llvm/unittests/Target/RISCV/RISCVInstrInfoTest.cpp
@@ -161,6 +161,52 @@ TEST_P(RISCVInstrInfoTest, IsCopyInstrImpl) {
EXPECT_EQ(MI7Res->Source->getReg(), RISCV::X2);
}
+TEST_P(RISCVInstrInfoTest, GetConstValDefinedInReg) {
+ const RISCVInstrInfo *TII = ST->getInstrInfo();
+ DebugLoc DL;
+ int64_t ImmVal;
+
+ auto *MI1 = BuildMI(*MF, DL, TII->get(RISCV::ADD), RISCV::X1)
+ .addReg(RISCV::X2)
+ .addReg(RISCV::X3)
+ .getInstr();
+ EXPECT_FALSE(TII->getConstValDefinedInReg(*MI1, RISCV::X1, ImmVal));
+
+ auto *MI2 = BuildMI(*MF, DL, TII->get(RISCV::ADDI), RISCV::X1)
+ .addReg(RISCV::X0)
+ .addImm(-128)
+ .getInstr();
+ EXPECT_FALSE(TII->getConstValDefinedInReg(*MI2, RISCV::X0, ImmVal));
+ ASSERT_TRUE(TII->getConstValDefinedInReg(*MI2, RISCV::X1, ImmVal));
+ EXPECT_EQ(ImmVal, -128);
+
+ auto *MI3 = BuildMI(*MF, DL, TII->get(RISCV::ORI), RISCV::X2)
+ .addReg(RISCV::X0)
+ .addImm(1024)
+ .getInstr();
+ EXPECT_FALSE(TII->getConstValDefinedInReg(*MI3, RISCV::X0, ImmVal));
+ ASSERT_TRUE(TII->getConstValDefinedInReg(*MI3, RISCV::X2, ImmVal));
+ EXPECT_EQ(ImmVal, 1024);
+
+ if (ST->is64Bit()) {
+ auto *MI4 = BuildMI(*MF, DL, TII->get(RISCV::ADDIW), RISCV::X2)
+ .addReg(RISCV::X0)
+ .addImm(512)
+ .getInstr();
+ EXPECT_FALSE(TII->getConstValDefinedInReg(*MI4, RISCV::X0, ImmVal));
+ ASSERT_TRUE(TII->getConstValDefinedInReg(*MI4, RISCV::X2, ImmVal));
+ EXPECT_EQ(ImmVal, 512);
+ }
+
+ auto *MI5 = BuildMI(*MF, DL, TII->get(RISCV::ADD), RISCV::X1)
+ .addReg(RISCV::X0)
+ .addReg(RISCV::X0)
+ .getInstr();
+ EXPECT_FALSE(TII->getConstValDefinedInReg(*MI5, RISCV::X0, ImmVal));
+ ASSERT_TRUE(TII->getConstValDefinedInReg(*MI5, RISCV::X1, ImmVal));
+ EXPECT_EQ(ImmVal, 0);
+}
+
TEST_P(RISCVInstrInfoTest, GetMemOperandsWithOffsetWidth) {
const RISCVInstrInfo *TII = ST->getInstrInfo();
const TargetRegisterInfo *TRI = ST->getRegisterInfo();
More information about the llvm-commits
mailing list