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

[DavidSpickett]

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

252

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[] = {
     {"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},
+    // 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;
+  RISCVInstrType 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) {
       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>;
 
+enum RISCVInstrType {
+  RV32,
+  RV64,
+  RV128,
+};
+
 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);
+  /// We suppose the default of `inst_type` is RV64.
+  RISCVInstrType inst_type = RV64;
 };
 
 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)