[llvm] [RISCV] Implement RISCVInstrInfo::isAddImmediate (PR #72356)

via llvm-commits llvm-commits at lists.llvm.org
Tue Nov 14 22:58:20 PST 2023


llvmbot wrote:


<!--LLVM PR SUMMARY COMMENT-->

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

Author: Alex Bradbury (asb)

<details>
<summary>Changes</summary>

This hook is called by the target-independent implementation of TargetInstrInfo::describeLoadedValue. I've opted to test it via a C++ unit test, which although fiddly to set up seems the right way to test a function with such clear intended semantics (rather than testing the impact indirectly).

isAddImmediate will never recognise ADDIW as an add immediate which I _think_ is conservatively correct, as the caller may not understand its semantics vs ADDI.

Note that although the doc comment for isAddImmediate specifies its behaviuor solely in terms of physical registers, none of the current in-tree implementations (including this one) bail out on virtual registers.

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


4 Files Affected:

- (modified) llvm/lib/Target/RISCV/RISCVInstrInfo.cpp (+17) 
- (modified) llvm/lib/Target/RISCV/RISCVInstrInfo.h (+3) 
- (modified) llvm/unittests/Target/RISCV/CMakeLists.txt (+5-1) 
- (added) llvm/unittests/Target/RISCV/RISCVInstrInfoTest.cpp (+96) 


``````````diff
diff --git a/llvm/lib/Target/RISCV/RISCVInstrInfo.cpp b/llvm/lib/Target/RISCV/RISCVInstrInfo.cpp
index 9271f807a84838b..45df0b111752c21 100644
--- a/llvm/lib/Target/RISCV/RISCVInstrInfo.cpp
+++ b/llvm/lib/Target/RISCV/RISCVInstrInfo.cpp
@@ -2438,6 +2438,23 @@ MachineBasicBlock::iterator RISCVInstrInfo::insertOutlinedCall(
   return It;
 }
 
+std::optional<RegImmPair> RISCVInstrInfo::isAddImmediate(const MachineInstr &MI,
+                                                         Register Reg) const {
+  // TODO: Handle cases where Reg is a super- or sub-register of the
+  // destination register.
+  const MachineOperand &Op0 = MI.getOperand(0);
+  if (!Op0.isReg() || Reg != Op0.getReg())
+    return std::nullopt;
+
+  // Don't consider ADDIW as a candidate because the caller may not be aware
+  // of its sign extension behaviour.
+  if (MI.getOpcode() == RISCV::ADDI && MI.getOperand(1).isReg() &&
+      MI.getOperand(2).isImm())
+    return RegImmPair{MI.getOperand(1).getReg(), MI.getOperand(2).getImm()};
+
+  return std::nullopt;
+}
+
 // 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 b33d8c28561596b..c2bffe1f9d6a1bf 100644
--- a/llvm/lib/Target/RISCV/RISCVInstrInfo.h
+++ b/llvm/lib/Target/RISCV/RISCVInstrInfo.h
@@ -197,6 +197,9 @@ class RISCVInstrInfo : public RISCVGenInstrInfo {
                      MachineBasicBlock::iterator &It, MachineFunction &MF,
                      outliner::Candidate &C) const override;
 
+  std::optional<RegImmPair> isAddImmediate(const MachineInstr &MI,
+                                           Register Reg) 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/CMakeLists.txt b/llvm/unittests/Target/RISCV/CMakeLists.txt
index 9d0bf7244c02210..d80d29b7f0dadac 100644
--- a/llvm/unittests/Target/RISCV/CMakeLists.txt
+++ b/llvm/unittests/Target/RISCV/CMakeLists.txt
@@ -7,13 +7,17 @@ set(LLVM_LINK_COMPONENTS
   RISCVCodeGen
   RISCVDesc
   RISCVInfo
-  TargetParser
+  CodeGen
+  Core
   MC
+  SelectionDAG
+  TargetParser
   )
 
 add_llvm_target_unittest(RISCVTests
   MCInstrAnalysisTest.cpp
   RISCVBaseInfoTest.cpp
+  RISCVInstrInfoTest.cpp
   )
 
 set_property(TARGET RISCVTests PROPERTY FOLDER "Tests/UnitTests/TargetTests")
diff --git a/llvm/unittests/Target/RISCV/RISCVInstrInfoTest.cpp b/llvm/unittests/Target/RISCV/RISCVInstrInfoTest.cpp
new file mode 100644
index 000000000000000..1e9a259a6b3f110
--- /dev/null
+++ b/llvm/unittests/Target/RISCV/RISCVInstrInfoTest.cpp
@@ -0,0 +1,96 @@
+//===- RISCVInstrInfoTest.cpp - RISCVInstrInfo unit tests -----------------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#include "RISCVInstrInfo.h"
+#include "RISCVSubtarget.h"
+#include "RISCVTargetMachine.h"
+#include "llvm/CodeGen/MachineModuleInfo.h"
+#include "llvm/MC/TargetRegistry.h"
+#include "llvm/Support/TargetSelect.h"
+#include "llvm/Target/TargetMachine.h"
+#include "llvm/Target/TargetOptions.h"
+
+#include "gtest/gtest.h"
+
+#include <memory>
+
+using namespace llvm;
+
+namespace {
+
+class RISCVInstrInfoTest : public testing::TestWithParam<const char *> {
+protected:
+  std::unique_ptr<LLVMContext> Ctx;
+  std::unique_ptr<RISCVSubtarget> ST;
+  std::unique_ptr<MachineModuleInfo> MMI;
+  std::unique_ptr<MachineFunction> MF;
+
+  static void SetUpTestSuite() {
+    LLVMInitializeRISCVTargetInfo();
+    LLVMInitializeRISCVTarget();
+    LLVMInitializeRISCVTargetMC();
+  }
+
+  RISCVInstrInfoTest() {
+    std::string Error;
+    auto TT(Triple::normalize(GetParam()));
+    const Target *TheTarget = TargetRegistry::lookupTarget(TT, Error);
+    TargetOptions Options;
+
+    RISCVTargetMachine *TM = static_cast<RISCVTargetMachine *>(
+        TheTarget->createTargetMachine(TT, "generic", "", Options, std::nullopt,
+                                       std::nullopt, CodeGenOptLevel::Default));
+
+    Ctx = std::make_unique<LLVMContext>();
+    Module M("Module", *Ctx);
+    M.setDataLayout(TM->createDataLayout());
+    auto *FType = FunctionType::get(Type::getVoidTy(*Ctx), false);
+    auto *F = Function::Create(FType, GlobalValue::ExternalLinkage, "Test", &M);
+    MMI = std::make_unique<MachineModuleInfo>(TM);
+
+    ST = std::make_unique<RISCVSubtarget>(
+        TM->getTargetTriple(), TM->getTargetCPU(), TM->getTargetCPU(),
+        TM->getTargetFeatureString(),
+        TM->getTargetTriple().isArch64Bit() ? "lp64" : "ilp32", 0, 0, *TM);
+
+    MF = std::make_unique<MachineFunction>(*F, *TM, *ST, 42, *MMI);
+  }
+};
+
+TEST_P(RISCVInstrInfoTest, IsAddImmediate) {
+  const RISCVInstrInfo *TII = ST->getInstrInfo();
+  DebugLoc DL;
+
+  MachineInstr *MI1 = BuildMI(*MF, DL, TII->get(RISCV::ADDI), RISCV::X1)
+                          .addReg(RISCV::X2)
+                          .addImm(-128)
+                          .getInstr();
+  auto MI1Res = TII->isAddImmediate(*MI1, RISCV::X1);
+  ASSERT_TRUE(MI1Res.has_value());
+  EXPECT_EQ(MI1Res->Reg, RISCV::X2);
+  EXPECT_EQ(MI1Res->Imm, -128);
+  EXPECT_FALSE(TII->isAddImmediate(*MI1, RISCV::X2).has_value());
+
+  MachineInstr *MI2 =
+      BuildMI(*MF, DL, TII->get(RISCV::LUI), RISCV::X1).addImm(-128).getInstr();
+  EXPECT_FALSE(TII->isAddImmediate(*MI2, RISCV::X1));
+
+  // Check ADDIW isn't treated as isAddImmediate.
+  if (ST->is64Bit()) {
+    MachineInstr *MI3 = BuildMI(*MF, DL, TII->get(RISCV::ADDIW), RISCV::X1)
+                            .addReg(RISCV::X2)
+                            .addImm(-128)
+                            .getInstr();
+    EXPECT_FALSE(TII->isAddImmediate(*MI3, RISCV::X1));
+  }
+}
+
+} // namespace
+
+INSTANTIATE_TEST_SUITE_P(RV32And64, RISCVInstrInfoTest,
+                         testing::Values("riscv32", "riscv64"));

``````````

</details>


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


More information about the llvm-commits mailing list