[llvm] [RISCV][test] Add tests for RISCVInstrInfo::describeLoadedValue (PR #76041)

Alex Bradbury via llvm-commits llvm-commits at lists.llvm.org
Wed Dec 20 03:18:27 PST 2023


https://github.com/asb created https://github.com/llvm/llvm-project/pull/76041

Tests are in preparation for adding handling of the load of a constant value as Mips does (noted in
<https://github.com/llvm/llvm-project/pull/72356#discussion_r1395203532>).

I've opted to implement these tests as a C++ unit test as on balance I _think_ it's easier to follow and maintain than .mir tests trying to indirectly test this function. That said, you see the limitations with the test of describeLoadedValue on a memory operation where we'd rather pass `MachinePointerInfo::getFixedStack` but can't because we'd need to then ensure the necessary stack metadata for the function is present.

>From cb3c32cec0c8b41712bbe8222cb354284e6796eb Mon Sep 17 00:00:00 2001
From: Alex Bradbury <asb at igalia.com>
Date: Wed, 20 Dec 2023 11:11:15 +0000
Subject: [PATCH] [RISCV][test] Add tests for
 RISCVInstrInfo::describeLoadedValue

Tests are in preparation for adding handling of the load of a constant
value as Mips does (noted in
<https://github.com/llvm/llvm-project/pull/72356#discussion_r1395203532>).

I've opted to implement these tests as a C++ unit test as on balance I
_think_ it's easier to follow and maintain than .mir tests trying to
indirectly test this function. That said, you see the limitations with
the test of describeLoadedValue on a memory operation where we'd rather
pass `MachinePointerInfo::getFixedStack` but can't because we'd need to
then ensure the necessary stack metadata for the function is present.
---
 .../Target/RISCV/RISCVInstrInfoTest.cpp       | 78 +++++++++++++++++++
 1 file changed, 78 insertions(+)

diff --git a/llvm/unittests/Target/RISCV/RISCVInstrInfoTest.cpp b/llvm/unittests/Target/RISCV/RISCVInstrInfoTest.cpp
index 5ef86bb3f7b46e..5836239bc56fd6 100644
--- a/llvm/unittests/Target/RISCV/RISCVInstrInfoTest.cpp
+++ b/llvm/unittests/Target/RISCV/RISCVInstrInfoTest.cpp
@@ -10,6 +10,7 @@
 #include "RISCVSubtarget.h"
 #include "RISCVTargetMachine.h"
 #include "llvm/CodeGen/MachineModuleInfo.h"
+#include "llvm/IR/DebugInfoMetadata.h"
 #include "llvm/MC/TargetRegistry.h"
 #include "llvm/Support/TargetSelect.h"
 #include "llvm/Target/TargetLoweringObjectFile.h"
@@ -174,6 +175,83 @@ TEST_P(RISCVInstrInfoTest, GetMemOperandsWithOffsetWidth) {
   EXPECT_EQ(Width, 4u);
 }
 
+static void expectDIEPrintResult(const DIExpression *Expr, StringRef Expected) {
+  std::string Output;
+  raw_string_ostream OS(Output);
+  Expr->print(OS);
+  OS.flush();
+  EXPECT_EQ(OS.str(), Expected);
+}
+
+TEST_P(RISCVInstrInfoTest, DescribeLoadedValue) {
+  const RISCVInstrInfo *TII = ST->getInstrInfo();
+  DebugLoc DL;
+
+  MachineBasicBlock *MBB = MF->CreateMachineBasicBlock();
+  MF->getProperties().set(MachineFunctionProperties::Property::NoVRegs);
+
+  // Register move.
+  auto *MI1 = BuildMI(*MBB, MBB->begin(), DL, TII->get(RISCV::ADDI), RISCV::X1)
+                  .addReg(RISCV::X2)
+                  .addImm(0)
+                  .getInstr();
+  EXPECT_FALSE(TII->describeLoadedValue(*MI1, RISCV::X2).has_value());
+  std::optional<ParamLoadedValue> MI1Res =
+      TII->describeLoadedValue(*MI1, RISCV::X1);
+  ASSERT_TRUE(MI1Res.has_value());
+  ASSERT_TRUE(MI1Res->first.isReg());
+  EXPECT_EQ(MI1Res->first.getReg(), RISCV::X2);
+  expectDIEPrintResult(MI1Res->second, "!DIExpression()");
+
+  // Load immediate.
+  auto *MI2 = BuildMI(*MBB, MBB->begin(), DL, TII->get(RISCV::ADDI), RISCV::X3)
+                  .addReg(RISCV::X0)
+                  .addImm(111)
+                  .getInstr();
+  std::optional<ParamLoadedValue> MI2Res =
+      TII->describeLoadedValue(*MI2, RISCV::X3);
+  ASSERT_TRUE(MI2Res.has_value());
+  ASSERT_TRUE(MI2Res->first.isReg());
+  EXPECT_EQ(MI2Res->first.getReg(), RISCV::X0);
+  // TODO: Could be a DW_OP_constu if this is recognised as a immediate load
+  // rather than just an addi.
+  expectDIEPrintResult(MI2Res->second, "!DIExpression(DW_OP_plus_uconst, 111)");
+
+  // Add immediate.
+  auto *MI3 = BuildMI(*MBB, MBB->begin(), DL, TII->get(RISCV::ADDI), RISCV::X2)
+                  .addReg(RISCV::X3)
+                  .addImm(222)
+                  .getInstr();
+  std::optional<ParamLoadedValue> MI3Res =
+      TII->describeLoadedValue(*MI3, RISCV::X2);
+  ASSERT_TRUE(MI3Res.has_value());
+  ASSERT_TRUE(MI3Res->first.isReg());
+  EXPECT_EQ(MI3Res->first.getReg(), RISCV::X3);
+  expectDIEPrintResult(MI3Res->second, "!DIExpression(DW_OP_plus_uconst, 222)");
+
+  // Load value from memory.
+  // It would be better (more reflective of real-world describeLoadedValue
+  // usage) to test using MachinePointerInfo::getFixedStack, but
+  // unfortunately it would be overly fiddly to make this work.
+  auto MMO = MF->getMachineMemOperand(MachinePointerInfo::getConstantPool(*MF),
+                                      MachineMemOperand::MOLoad, 1, Align(1));
+  auto *MI4 = BuildMI(*MBB, MBB->begin(), DL, TII->get(RISCV::LB), RISCV::X1)
+                  .addReg(RISCV::X2)
+                  .addImm(-128)
+                  .addMemOperand(MMO)
+                  .getInstr();
+  std::optional<ParamLoadedValue> MI4Res =
+      TII->describeLoadedValue(*MI4, RISCV::X1);
+  ASSERT_TRUE(MI4Res.has_value());
+  ASSERT_TRUE(MI4Res->first.isReg());
+  EXPECT_EQ(MI4Res->first.getReg(), RISCV::X2);
+  expectDIEPrintResult(
+      MI4Res->second,
+      "!DIExpression(DW_OP_constu, 128, DW_OP_minus, DW_OP_deref_size, 1)");
+
+  MF->deleteMachineBasicBlock(MBB);
+}
+
 } // namespace
 
 INSTANTIATE_TEST_SUITE_P(RV32And64, RISCVInstrInfoTest,



More information about the llvm-commits mailing list