[LLDB][RISCV] Add RV32FC instruction support for EmulateInstructionRISCV

[DavidSpickett]

lldb/source/Plugins/Instruction/RISCV/RISCVInstructions.h

250

I assume this means whether we're 64 bit or 32 bit only? I'd prefer an enum that encodes the choices rather than a number if so.

Or an bool 64_bit_only if that makes more sense in context.

[Emmmer]

address review comments.

[DavidSpickett]

How do the RV32/RV64 instructions split? Is it the case that the majority are one or the other, or that the majority are both and only a few will be 32 or 64 specific?

I expected to see a "both" in the enum or something saying "RV64 can be either", maybe I just missed it.

[Emmmer]

I think this will be helpful.

[DavidSpickett]

Yes that's much clearer. LGTM.

lldb/source/Plugins/Instruction/RISCV/EmulateInstructionRISCV.cpp

static const InstrPattern PATTERNS[] = {
     {"AMOOR_D", 0xF800707F, 0x4000302F, DecodeRType<AMOOR_D>},
     {"AMOMIN_D", 0xF800707F, 0x8000302F, DecodeRType<AMOMIN_D>},
     {"AMOMAX_D", 0xF800707F, 0xA000302F, DecodeRType<AMOMAX_D>},
     {"AMOMINU_D", 0xF800707F, 0xC000302F, DecodeRType<AMOMINU_D>},
     {"AMOMAXU_D", 0xF800707F, 0xE000302F, DecodeRType<AMOMAXU_D>},
 
     // RVC (Compressed Instructions) //
     {"C_LWSP", 0xE003, 0x4002, DecodeC_LWSP},
-    {"C_LDSP", 0xE003, 0x6002, DecodeC_LDSP},
+    {"C_LDSP", 0xE003, 0x6002, DecodeC_LDSP, RV64 | RV128},
     {"C_SWSP", 0xE003, 0xC002, DecodeC_SWSP},
-    {"C_SDSP", 0xE003, 0xE002, DecodeC_SDSP},
+    {"C_SDSP", 0xE003, 0xE002, DecodeC_SDSP, RV64 | RV128},
     {"C_LW", 0xE003, 0x4000, DecodeC_LW},
-    {"C_LD", 0xE003, 0x6000, DecodeC_LD},
+    {"C_LD", 0xE003, 0x6000, DecodeC_LD, RV64 | RV128},
     {"C_SW", 0xE003, 0xC000, DecodeC_SW},
-    {"C_SD", 0xE003, 0xE000, DecodeC_SD},
+    {"C_SD", 0xE003, 0xE000, DecodeC_SD, RV64 | RV128},
     {"C_J", 0xE003, 0xA001, DecodeC_J},
     {"C_JR", 0xF07F, 0x8002, DecodeC_JR},
     {"C_JALR", 0xF07F, 0x9002, DecodeC_JALR},
     {"C_BNEZ", 0xE003, 0xE001, DecodeC_BNEZ},
     {"C_BEQZ", 0xE003, 0xC001, DecodeC_BEQZ},
     {"C_LI", 0xE003, 0x4001, DecodeC_LI},
     {"C_LUI_ADDI16SP", 0xE003, 0x6001, DecodeC_LUI_ADDI16SP},
     {"C_ADDI", 0xE003, 0x1, DecodeC_ADDI},
-    {"C_ADDIW", 0xE003, 0x2001, DecodeC_ADDIW},
+    {"C_ADDIW", 0xE003, 0x2001, DecodeC_ADDIW, RV64 | RV128},
     {"C_ADDI4SPN", 0xE003, 0x0, DecodeC_ADDI4SPN},
-    {"C_SLLI", 0xE003, 0x2, DecodeC_SLLI},
-    {"C_SRLI", 0xEC03, 0x8001, DecodeC_SRLI},
-    {"C_SRAI", 0xEC03, 0x8401, DecodeC_SRAI},
+    {"C_SLLI", 0xE003, 0x2, DecodeC_SLLI, RV64 | RV128},
+    {"C_SRLI", 0xEC03, 0x8001, DecodeC_SRLI, RV64 | RV128},
+    {"C_SRAI", 0xEC03, 0x8401, DecodeC_SRAI, RV64 | RV128},
     {"C_ANDI", 0xEC03, 0x8801, DecodeC_ANDI},
     {"C_MV", 0xF003, 0x8002, DecodeC_MV},
     {"C_ADD", 0xF003, 0x9002, DecodeC_ADD},
     {"C_AND", 0xFC63, 0x8C61, DecodeC_AND},
     {"C_OR", 0xFC63, 0x8C41, DecodeC_OR},
     {"C_XOR", 0xFC63, 0x8C21, DecodeC_XOR},
     {"C_SUB", 0xFC63, 0x8C01, DecodeC_SUB},
-    {"C_SUBW", 0xFC63, 0x9C01, DecodeC_SUBW},
-    {"C_ADDW", 0xFC63, 0x9C21, DecodeC_ADDW},
+    {"C_SUBW", 0xFC63, 0x9C01, DecodeC_SUBW, RV64 | RV128},
+    {"C_ADDW", 0xFC63, 0x9C21, DecodeC_ADDW, RV64 | RV128},
+    // RV32FC //
+    {"FLW", 0xE003, 0x6000, DecodeC_FLW, RV32},
+    {"FSW", 0xE003, 0xE000, DecodeC_FSW, RV32},
+    {"FLWSP", 0xE003, 0x6002, DecodeC_FLWSP, RV32},
+    {"FSWSP", 0xE003, 0xE002, DecodeC_FSWSP, RV32},
 
     // RV32F (Extension for Single-Precision Floating-Point) //
     {"FLW", 0x707F, 0x2007, DecodeIType<FLW>},
     {"FSW", 0x707F, 0x2027, DecodeSType<FSW>},
     {"FMADD_S", 0x600007F, 0x43, DecodeR4Type<FMADD_S>},
     {"FMSUB_S", 0x600007F, 0x47, DecodeR4Type<FMSUB_S>},
     {"FNMSUB_S", 0x600007F, 0x4B, DecodeR4Type<FNMSUB_S>},
     {"FNMADD_S", 0x600007F, 0x4F, DecodeR4Type<FNMADD_S>},

 
 llvm::Optional<DecodeResult> EmulateInstructionRISCV::Decode(uint32_t inst) {
   Log *log = GetLog(LLDBLog::Unwind);
 
   uint16_t try_rvc = uint16_t(inst & 0x0000ffff);
   // check whether the compressed encode could be valid
   uint16_t mask = try_rvc & 0b11;
   bool is_rvc = try_rvc != 0 && mask != 3;
+  uint8_t inst_type = RV64;
+
+  // if we have ArchSpec::eCore_riscv128 in the future,
+  // we also need to check it here
+  if (m_arch.GetCore() == ArchSpec::eCore_riscv32)
+    inst_type = RV32;
 
   for (const InstrPattern &pat : PATTERNS) {
-    if ((inst & pat.type_mask) == pat.eigen) {
+    if ((inst & pat.type_mask) == pat.eigen &&
+        (inst_type & pat.inst_type) != 0) {
       LLDB_LOGF(
           log, "EmulateInstructionRISCV::%s: inst(%x at %lx) was decoded to %s",
           __FUNCTION__, inst, m_addr, pat.name);
       auto decoded = is_rvc ? pat.decode(try_rvc) : pat.decode(inst);
       return DecodeResult{decoded, inst, is_rvc, pat};
     }
   }
   LLDB_LOGF(log, "EmulateInstructionRISCV::%s: inst(0x%x) was unsupported",

lldb/source/Plugins/Instruction/RISCV/RISCVCInstructions.h

RISCVInst DecodeC_SUBW(uint32_t inst) {
   auto rd = DecodeCA_RD(inst);
   return SUBW{rd, rd, DecodeCA_RS2(inst)};
 }
 
 RISCVInst DecodeC_ADDW(uint32_t inst) {
   auto rd = DecodeCA_RD(inst);
   return ADDW{rd, rd, DecodeCA_RS2(inst)};
 }
+RISCVInst DecodeC_FLW(uint32_t inst) {
+  uint16_t offset = ((inst << 1) & 0x40)   // imm[6]
+                    | ((inst >> 7) & 0x38) // imm[5:3]
+                    | ((inst >> 4) & 0x4); // imm[2]
+  return FLW{DecodeCL_RD(inst), DecodeCL_RS1(inst), uint32_t(offset)};
+}
+
+RISCVInst DecodeC_FSW(uint32_t inst) {
+  uint16_t offset = ((inst << 1) & 0x40)   // imm[6]
+                    | ((inst >> 7) & 0x38) // imm[5:3]
+                    | ((inst >> 4) & 0x4); // imm[2]
+  return FSW{DecodeCS_RS1(inst), DecodeCS_RS2(inst), uint32_t(offset)};
+}
+
+RISCVInst DecodeC_FLWSP(uint32_t inst) {
+  auto rd = DecodeCI_RD(inst);
+  uint16_t offset = ((inst << 4) & 0xc0)    // offset[7:6]
+                    | ((inst >> 7) & 0x20)  // offset[5]
+                    | ((inst >> 2) & 0x1c); // offset[4:2]
+  return FLW{rd, Rs{gpr_sp_riscv}, uint32_t(offset)};
+}
+
+RISCVInst DecodeC_FSWSP(uint32_t inst) {
+  uint16_t offset = ((inst >> 1) & 0xc0)    // offset[7:6]
+                    | ((inst >> 7) & 0x3c); // offset[5:2]
+  return FSW{Rs{gpr_sp_riscv}, DecodeCSS_RS2(inst), uint32_t(offset)};
+}
 
 } // namespace lldb_private
 #endif // LLDB_SOURCE_PLUGINS_INSTRUCTION_RISCV_RISCVCINSTRUCTION_H

lldb/source/Plugins/Instruction/RISCV/RISCVInstructions.h

using RISCVInst = std::variant<
     AMOOR_W, AMOMIN_W, AMOMAX_W, AMOMINU_W, AMOMAXU_W, LR_D, SC_D, AMOSWAP_D,
     AMOADD_D, AMOXOR_D, AMOAND_D, AMOOR_D, AMOMIN_D, AMOMAX_D, AMOMINU_D,
     AMOMAXU_D, FLW, FSW, FMADD_S, FMSUB_S, FNMADD_S, FNMSUB_S, FADD_S, FSUB_S,
     FMUL_S, FDIV_S, FSQRT_S, FSGNJ_S, FSGNJN_S, FSGNJX_S, FMIN_S, FMAX_S,
     FCVT_W_S, FCVT_WU_S, FMV_X_W, FEQ_S, FLT_S, FLE_S, FCLASS_S, FCVT_S_W,
     FCVT_S_WU, FMV_W_X, FCVT_L_S, FCVT_LU_S, FCVT_S_L, FCVT_S_LU, INVALID,
     EBREAK, RESERVED, HINT, NOP>;
 
+constexpr uint8_t RV32 = 1;
+constexpr uint8_t RV64 = 2;
+constexpr uint8_t RV128 = 4;
+
 struct InstrPattern {
   const char *name;
   /// Bit mask to check the type of a instruction (B-Type, I-Type, J-Type, etc.)
   uint32_t type_mask;
   /// Characteristic value after bitwise-and with type_mask.
   uint32_t eigen;
   RISCVInst (*decode)(uint32_t inst);
+  /// If not specified, the inst will be supported by all RV versions.
+  uint8_t inst_type = RV32 | RV64 | RV128;
 };
 
 struct DecodeResult {
   RISCVInst decoded;
   uint32_t inst;
   bool is_rvc;
   InstrPattern pattern;
 };

lldb/unittests/Instruction/RISCV/TestRISCVEmulator.cpp

 using namespace lldb;
 using namespace lldb_private;
 
 struct RISCVEmulatorTester : public EmulateInstructionRISCV, testing::Test {
   RegisterInfoPOSIX_riscv64::GPR gpr;
   RegisterInfoPOSIX_riscv64::FPR fpr;
   uint8_t memory[1024] = {0};
 
-  RISCVEmulatorTester()
-      : EmulateInstructionRISCV(ArchSpec("riscv64-unknown-linux-gnu")) {
+  RISCVEmulatorTester(std::string triple = "riscv64-unknown-linux-gnu")
+      : EmulateInstructionRISCV(ArchSpec(triple)) {
     EmulateInstruction::SetReadRegCallback(ReadRegisterCallback);
     EmulateInstruction::SetWriteRegCallback(WriteRegisterCallback);
     EmulateInstruction::SetReadMemCallback(ReadMemoryCallback);
     EmulateInstruction::SetWriteMemCallback(WriteMemoryCallback);
     ClearAll();
   }
 
   static bool ReadRegisterCallback(EmulateInstruction *instruction, void *baton,
TEST_F(RISCVEmulatorTester, TestCDecode) {
 
   for (auto i : tests) {
     auto decode = this->Decode(i.inst);
     ASSERT_TRUE(decode.has_value());
     ASSERT_TRUE(compareInst(decode->decoded, i.inst_type));
   }
 }
 
+class RISCVEmulatorTester32 : public RISCVEmulatorTester {
+public:
+  RISCVEmulatorTester32() : RISCVEmulatorTester("riscv32-unknown-linux-gnu") {}
+};
+
+TEST_F(RISCVEmulatorTester32, TestCDecodeRV32) {
+  std::vector<TestDecode> tests = {
+      {0x6002, FLW{Rd{0}, Rs{2}, 0}},
+      {0xE006, FSW{Rs{2}, Rs{1}, 0}},
+      {0x6000, FLW{Rd{8}, Rs{8}, 0}},
+      {0xE000, FSW{Rs{8}, Rs{8}, 0}},
+  };
+
+  for (auto i : tests) {
+    auto decode = this->Decode(i.inst);
+    ASSERT_TRUE(decode.has_value());
+    ASSERT_TRUE(compareInst(decode->decoded, i.inst_type));
+  }
+}
+
 // GEN_BRANCH_TEST(opcode, imm1, imm2, imm3):
 // It should branch for instruction `opcode imm1, imm2`
 // It should do nothing for instruction `opcode imm1, imm3`
 GEN_BRANCH_TEST(BEQ, 1, 1, 0)
 GEN_BRANCH_TEST(BNE, 1, 0, 1)
 GEN_BRANCH_TEST(BLT, -2, 1, -3)
 GEN_BRANCH_TEST(BGE, -2, -3, 1)
 GEN_BRANCH_TEST(BLTU, -2, -1, 1)