[llvm] 7f28e8c - [RISCV] Implement RISCVInstrInfo::isAddImmediate (#72356)

via llvm-commits llvm-commits at lists.llvm.org
Thu Nov 16 06:43:35 PST 2023


Author: Alex Bradbury
Date: 2023-11-16T14:43:31Z
New Revision: 7f28e8ced7ef36a51e5cbca08818288ab167a8ab

URL: https://github.com/llvm/llvm-project/commit/7f28e8ced7ef36a51e5cbca08818288ab167a8ab
DIFF: https://github.com/llvm/llvm-project/commit/7f28e8ced7ef36a51e5cbca08818288ab167a8ab.diff

LOG: [RISCV] Implement RISCVInstrInfo::isAddImmediate (#72356)

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
behaviour solely in terms of physical registers, none of the current
in-tree implementations (including this one) bail out on virtual
registers (see #72357).

Added: 
    llvm/unittests/Target/RISCV/RISCVInstrInfoTest.cpp

Modified: 
    llvm/lib/Target/RISCV/RISCVInstrInfo.cpp
    llvm/lib/Target/RISCV/RISCVInstrInfo.h
    llvm/unittests/Target/RISCV/CMakeLists.txt

Removed: 
    


################################################################################
diff  --git a/llvm/lib/Target/RISCV/RISCVInstrInfo.cpp b/llvm/lib/Target/RISCV/RISCVInstrInfo.cpp
index 5495d323bf17ebe..67658aa1dfb6b28 100644
--- a/llvm/lib/Target/RISCV/RISCVInstrInfo.cpp
+++ b/llvm/lib/Target/RISCV/RISCVInstrInfo.cpp
@@ -2441,6 +2441,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"));


        


More information about the llvm-commits mailing list