[lldb][RISCV] Support optionally disabled FPR for riscv64 #104547
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
@llvm/pr-subscribers-lldb Author: Alexey Merzlyakov (AlexeyMerzlyakov) ChangesThe PR adds the support optionally enabled/disabled FP-registers to LLDB To aim this, patch adds The patch flows are spread to
According testcase built for RV64IMAC (no-FPR) was added to Patch is 25.30 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/104547.diff 13 Files Affected:
diff --git a/lldb/source/Plugins/Instruction/RISCV/EmulateInstructionRISCV.cpp b/lldb/source/Plugins/Instruction/RISCV/EmulateInstructionRISCV.cpp
index e8014b1eeb3789..badc7ba36f0118 100644
--- a/lldb/source/Plugins/Instruction/RISCV/EmulateInstructionRISCV.cpp
+++ b/lldb/source/Plugins/Instruction/RISCV/EmulateInstructionRISCV.cpp
@@ -1748,10 +1748,10 @@ EmulateInstructionRISCV::GetRegisterInfo(RegisterKind reg_kind,
}
}
- const RegisterInfo *array =
- RegisterInfoPOSIX_riscv64::GetRegisterInfoPtr(m_arch);
- const uint32_t length =
- RegisterInfoPOSIX_riscv64::GetRegisterInfoCount(m_arch);
+ RegisterInfoPOSIX_riscv64 reg_info(m_arch,
+ RegisterInfoPOSIX_riscv64::eRegsetMaskAll);
+ const RegisterInfo *array = reg_info.GetRegisterInfo();
+ const uint32_t length = reg_info.GetRegisterCount();
if (reg_index >= length || reg_kind != eRegisterKindLLDB)
return {};
diff --git a/lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_riscv64.cpp b/lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_riscv64.cpp
index 1d51726a86df16..aa8e6a0869aacb 100644
--- a/lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_riscv64.cpp
+++ b/lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_riscv64.cpp
@@ -23,12 +23,11 @@
// System includes - They have to be included after framework includes because
// they define some macros which collide with variable names in other modules
+#include <sys/ptrace.h>
#include <sys/uio.h>
// NT_PRSTATUS and NT_FPREGSET definition
#include <elf.h>
-#define REG_CONTEXT_SIZE (GetGPRSize() + GetFPRSize())
-
using namespace lldb;
using namespace lldb_private;
using namespace lldb_private::process_linux;
@@ -38,7 +37,21 @@ NativeRegisterContextLinux::CreateHostNativeRegisterContextLinux(
const ArchSpec &target_arch, NativeThreadLinux &native_thread) {
switch (target_arch.GetMachine()) {
case llvm::Triple::riscv64: {
- Flags opt_regsets;
+ Flags opt_regsets(RegisterInfoPOSIX_riscv64::eRegsetMaskDefault);
+
+ RegisterInfoPOSIX_riscv64::FPR fpr;
+ struct iovec ioVec;
+ ioVec.iov_base = &fpr;
+ ioVec.iov_len = sizeof(fpr);
+ unsigned int regset = NT_FPREGSET;
+
+ if (NativeProcessLinux::PtraceWrapper(PTRACE_GETREGSET,
+ native_thread.GetID(), ®set,
+ &ioVec, sizeof(fpr))
+ .Success()) {
+ opt_regsets.Set(RegisterInfoPOSIX_riscv64::eRegsetMaskFP);
+ }
+
auto register_info_up =
std::make_unique<RegisterInfoPOSIX_riscv64>(target_arch, opt_regsets);
return std::make_unique<NativeRegisterContextLinux_riscv64>(
@@ -193,20 +206,23 @@ Status NativeRegisterContextLinux_riscv64::ReadAllRegisterValues(
lldb::WritableDataBufferSP &data_sp) {
Status error;
- data_sp.reset(new DataBufferHeap(REG_CONTEXT_SIZE, 0));
+ data_sp.reset(new DataBufferHeap(GetRegContextSize(), 0));
error = ReadGPR();
if (error.Fail())
return error;
- error = ReadFPR();
- if (error.Fail())
- return error;
+ if (GetRegisterInfo().IsFPPresent()) {
+ error = ReadFPR();
+ if (error.Fail())
+ return error;
+ }
uint8_t *dst = const_cast<uint8_t *>(data_sp->GetBytes());
::memcpy(dst, GetGPRBuffer(), GetGPRSize());
dst += GetGPRSize();
- ::memcpy(dst, GetFPRBuffer(), GetFPRSize());
+ if (GetRegisterInfo().IsFPPresent())
+ ::memcpy(dst, GetFPRBuffer(), GetFPRSize());
return error;
}
@@ -222,11 +238,11 @@ Status NativeRegisterContextLinux_riscv64::WriteAllRegisterValues(
return error;
}
- if (data_sp->GetByteSize() != REG_CONTEXT_SIZE) {
+ if (data_sp->GetByteSize() != GetRegContextSize()) {
error.SetErrorStringWithFormat(
"NativeRegisterContextLinux_riscv64::%s data_sp contained mismatched "
"data size, expected %" PRIu64 ", actual %" PRIu64,
- __FUNCTION__, REG_CONTEXT_SIZE, data_sp->GetByteSize());
+ __FUNCTION__, GetRegContextSize(), data_sp->GetByteSize());
return error;
}
@@ -245,23 +261,32 @@ Status NativeRegisterContextLinux_riscv64::WriteAllRegisterValues(
return error;
src += GetRegisterInfoInterface().GetGPRSize();
- ::memcpy(GetFPRBuffer(), src, GetFPRSize());
- error = WriteFPR();
- if (error.Fail())
- return error;
+ if (GetRegisterInfo().IsFPPresent()) {
+ ::memcpy(GetFPRBuffer(), src, GetFPRSize());
+
+ error = WriteFPR();
+ if (error.Fail())
+ return error;
+ }
return error;
}
+size_t NativeRegisterContextLinux_riscv64::GetRegContextSize() {
+ size_t size = GetGPRSize();
+ if (GetRegisterInfo().IsFPPresent())
+ size += GetFPRSize();
+ return size;
+}
+
bool NativeRegisterContextLinux_riscv64::IsGPR(unsigned reg) const {
return GetRegisterInfo().GetRegisterSetFromRegisterIndex(reg) ==
RegisterInfoPOSIX_riscv64::GPRegSet;
}
bool NativeRegisterContextLinux_riscv64::IsFPR(unsigned reg) const {
- return GetRegisterInfo().GetRegisterSetFromRegisterIndex(reg) ==
- RegisterInfoPOSIX_riscv64::FPRegSet;
+ return GetRegisterInfo().IsFPReg(reg);
}
Status NativeRegisterContextLinux_riscv64::ReadGPR() {
diff --git a/lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_riscv64.h b/lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_riscv64.h
index 41b4e2573add9c..d5cc50131cdc30 100644
--- a/lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_riscv64.h
+++ b/lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_riscv64.h
@@ -75,6 +75,8 @@ class NativeRegisterContextLinux_riscv64 : public NativeRegisterContextLinux {
RegisterInfoPOSIX_riscv64::FPR m_fpr;
+ size_t GetRegContextSize();
+
bool IsGPR(unsigned reg) const;
bool IsFPR(unsigned reg) const;
diff --git a/lldb/source/Plugins/Process/Utility/RegisterContextPOSIX_riscv64.cpp b/lldb/source/Plugins/Process/Utility/RegisterContextPOSIX_riscv64.cpp
index 035ce00e116269..bbcfb9eae10034 100644
--- a/lldb/source/Plugins/Process/Utility/RegisterContextPOSIX_riscv64.cpp
+++ b/lldb/source/Plugins/Process/Utility/RegisterContextPOSIX_riscv64.cpp
@@ -77,6 +77,5 @@ bool RegisterContextPOSIX_riscv64::IsGPR(unsigned int reg) {
}
bool RegisterContextPOSIX_riscv64::IsFPR(unsigned int reg) {
- return m_register_info_up->GetRegisterSetFromRegisterIndex(reg) ==
- RegisterInfoPOSIX_riscv64::FPRegSet;
+ return m_register_info_up->IsFPReg(reg);
}
diff --git a/lldb/source/Plugins/Process/Utility/RegisterInfoPOSIX_riscv64.cpp b/lldb/source/Plugins/Process/Utility/RegisterInfoPOSIX_riscv64.cpp
index 3819401c543b1b..4a3737795848e9 100644
--- a/lldb/source/Plugins/Process/Utility/RegisterInfoPOSIX_riscv64.cpp
+++ b/lldb/source/Plugins/Process/Utility/RegisterInfoPOSIX_riscv64.cpp
@@ -18,42 +18,15 @@
#define GPR_OFFSET(idx) ((idx)*8 + 0)
#define FPR_OFFSET(idx) ((idx)*8 + sizeof(RegisterInfoPOSIX_riscv64::GPR))
-#define REG_CONTEXT_SIZE \
- (sizeof(RegisterInfoPOSIX_riscv64::GPR) + \
- sizeof(RegisterInfoPOSIX_riscv64::FPR))
-
#define DECLARE_REGISTER_INFOS_RISCV64_STRUCT
#include "RegisterInfos_riscv64.h"
#undef DECLARE_REGISTER_INFOS_RISCV64_STRUCT
-const lldb_private::RegisterInfo *RegisterInfoPOSIX_riscv64::GetRegisterInfoPtr(
- const lldb_private::ArchSpec &target_arch) {
- switch (target_arch.GetMachine()) {
- case llvm::Triple::riscv64:
- return g_register_infos_riscv64_le;
- default:
- assert(false && "Unhandled target architecture.");
- return nullptr;
- }
-}
-
-uint32_t RegisterInfoPOSIX_riscv64::GetRegisterInfoCount(
- const lldb_private::ArchSpec &target_arch) {
- switch (target_arch.GetMachine()) {
- case llvm::Triple::riscv64:
- return static_cast<uint32_t>(sizeof(g_register_infos_riscv64_le) /
- sizeof(g_register_infos_riscv64_le[0]));
- default:
- assert(false && "Unhandled target architecture.");
- return 0;
- }
-}
-
// Number of register sets provided by this context.
enum {
k_num_gpr_registers = gpr_last_riscv - gpr_first_riscv + 1,
k_num_fpr_registers = fpr_last_riscv - fpr_first_riscv + 1,
- k_num_register_sets = 2
+ k_num_register_sets_default = 1
};
// RISC-V64 general purpose registers.
@@ -73,38 +46,69 @@ static_assert(((sizeof g_gpr_regnums_riscv64 /
1) == k_num_gpr_registers,
"g_gpr_regnums_riscv64 has wrong number of register infos");
-// RISC-V64 floating point registers.
-static const uint32_t g_fpr_regnums_riscv64[] = {
- fpr_f0_riscv, fpr_f1_riscv, fpr_f2_riscv, fpr_f3_riscv,
- fpr_f4_riscv, fpr_f5_riscv, fpr_f6_riscv, fpr_f7_riscv,
- fpr_f8_riscv, fpr_f9_riscv, fpr_f10_riscv, fpr_f11_riscv,
- fpr_f12_riscv, fpr_f13_riscv, fpr_f14_riscv, fpr_f15_riscv,
- fpr_f16_riscv, fpr_f17_riscv, fpr_f18_riscv, fpr_f19_riscv,
- fpr_f20_riscv, fpr_f21_riscv, fpr_f22_riscv, fpr_f23_riscv,
- fpr_f24_riscv, fpr_f25_riscv, fpr_f26_riscv, fpr_f27_riscv,
- fpr_f28_riscv, fpr_f29_riscv, fpr_f30_riscv, fpr_f31_riscv,
- fpr_fcsr_riscv, LLDB_INVALID_REGNUM};
-
-static_assert(((sizeof g_fpr_regnums_riscv64 /
- sizeof g_fpr_regnums_riscv64[0]) -
- 1) == k_num_fpr_registers,
- "g_fpr_regnums_riscv64 has wrong number of register infos");
-
// Register sets for RISC-V64.
-static const lldb_private::RegisterSet g_reg_sets_riscv64[k_num_register_sets] =
- {{"General Purpose Registers", "gpr", k_num_gpr_registers,
- g_gpr_regnums_riscv64},
- {"Floating Point Registers", "fpr", k_num_fpr_registers,
- g_fpr_regnums_riscv64}};
+static const lldb_private::RegisterSet g_reg_set_gpr_riscv64 = {
+ "General Purpose Registers", "gpr", k_num_gpr_registers,
+ g_gpr_regnums_riscv64};
+static const lldb_private::RegisterSet g_reg_set_fpr_riscv64 = {
+ "Floating Point Registers", "fpr", k_num_fpr_registers, nullptr};
RegisterInfoPOSIX_riscv64::RegisterInfoPOSIX_riscv64(
- const lldb_private::ArchSpec &target_arch, lldb_private::Flags flags)
+ const lldb_private::ArchSpec &target_arch, lldb_private::Flags opt_regsets)
: lldb_private::RegisterInfoAndSetInterface(target_arch),
- m_register_info_p(GetRegisterInfoPtr(target_arch)),
- m_register_info_count(GetRegisterInfoCount(target_arch)) {}
+ m_opt_regsets(opt_regsets) {
+ switch (target_arch.GetMachine()) {
+ case llvm::Triple::riscv64: {
+ // By-default considering RISC-V has only GPR.
+ // Other register sets could be enabled optionally by opt_regsets.
+ AddRegSetGP();
+
+ if (m_opt_regsets.AnySet(eRegsetMaskFP))
+ AddRegSetFP();
+
+ break;
+ }
+ default:
+ assert(false && "Unhandled target architecture.");
+ }
+}
+
+void RegisterInfoPOSIX_riscv64::AddRegSetGP() {
+ m_register_infos.resize(k_num_gpr_registers);
+ memcpy(&m_register_infos[0], g_register_infos_riscv64_gpr,
+ sizeof(g_register_infos_riscv64_gpr));
+ m_register_sets.push_back(g_reg_set_gpr_riscv64);
+
+ m_per_regset_regnum_range[GPRegSet] =
+ std::make_pair(gpr_first_riscv, m_register_infos.size());
+}
+
+void RegisterInfoPOSIX_riscv64::AddRegSetFP() {
+ const uint32_t register_info_count = m_register_infos.size();
+ const uint32_t register_set_count = m_register_sets.size();
+
+ // Filling m_register_infos.
+ // For FPR case we do not need to correct register offsets and kinds
+ // while for other further cases (like VPR), register offset/kind
+ // should be started counting from the last one in previously added
+ // regset. This is needed for the case e.g. when architecture has GPR + VPR
+ // sets only.
+ m_register_infos.resize(register_info_count + k_num_fpr_registers);
+ memcpy(&m_register_infos[register_info_count], g_register_infos_riscv64_fpr,
+ sizeof(g_register_infos_riscv64_fpr));
+
+ // Filling m_register_sets with enabled register set
+ for (uint32_t i = 0; i < k_num_fpr_registers; i++)
+ m_fp_regnum_collection.push_back(register_info_count + i);
+ m_register_sets.push_back(g_reg_set_fpr_riscv64);
+ m_register_sets.back().registers = m_fp_regnum_collection.data();
+
+ m_per_regset_regnum_range[register_set_count] =
+ std::make_pair(register_info_count, m_register_infos.size());
+}
uint32_t RegisterInfoPOSIX_riscv64::GetRegisterCount() const {
- return m_register_info_count;
+ return m_register_infos.size();
}
size_t RegisterInfoPOSIX_riscv64::GetGPRSize() const {
@@ -117,26 +121,30 @@ size_t RegisterInfoPOSIX_riscv64::GetFPRSize() const {
const lldb_private::RegisterInfo *
RegisterInfoPOSIX_riscv64::GetRegisterInfo() const {
- return m_register_info_p;
+ return m_register_infos.data();
}
size_t RegisterInfoPOSIX_riscv64::GetRegisterSetCount() const {
- return k_num_register_sets;
+ return m_register_sets.size();
}
size_t RegisterInfoPOSIX_riscv64::GetRegisterSetFromRegisterIndex(
uint32_t reg_index) const {
- // coverity[unsigned_compare]
- if (reg_index >= gpr_first_riscv && reg_index <= gpr_last_riscv)
- return GPRegSet;
- if (reg_index >= fpr_first_riscv && reg_index <= fpr_last_riscv)
- return FPRegSet;
+ for (const auto ®set_range : m_per_regset_regnum_range) {
+ if (reg_index >= regset_range.second.first &&
+ reg_index < regset_range.second.second)
+ return regset_range.first;
+ }
return LLDB_INVALID_REGNUM;
}
+bool RegisterInfoPOSIX_riscv64::IsFPReg(unsigned reg) const {
+ return llvm::is_contained(m_fp_regnum_collection, reg);
+}
+
const lldb_private::RegisterSet *
RegisterInfoPOSIX_riscv64::GetRegisterSet(size_t set_index) const {
if (set_index < GetRegisterSetCount())
- return &g_reg_sets_riscv64[set_index];
+ return &m_register_sets[set_index];
return nullptr;
}
diff --git a/lldb/source/Plugins/Process/Utility/RegisterInfoPOSIX_riscv64.h b/lldb/source/Plugins/Process/Utility/RegisterInfoPOSIX_riscv64.h
index 4bf4bede013284..f8e22c7df3c88f 100644
--- a/lldb/source/Plugins/Process/Utility/RegisterInfoPOSIX_riscv64.h
+++ b/lldb/source/Plugins/Process/Utility/RegisterInfoPOSIX_riscv64.h
@@ -11,19 +11,21 @@
#include "RegisterInfoAndSetInterface.h"
#include "lldb/Target/RegisterContext.h"
+#include "lldb/Utility/Flags.h"
#include "lldb/lldb-private.h"
#include <map>
class RegisterInfoPOSIX_riscv64
: public lldb_private::RegisterInfoAndSetInterface {
public:
- static const lldb_private::RegisterInfo *
- GetRegisterInfoPtr(const lldb_private::ArchSpec &target_arch);
- static uint32_t
- GetRegisterInfoCount(const lldb_private::ArchSpec &target_arch);
+ enum { GPRegSet = 0 };
-public:
- enum { GPRegSet = 0, FPRegSet };
+ // RISC-V64 register set mask value
+ enum {
+ eRegsetMaskDefault = 0,
+ eRegsetMaskFP = 1,
+ eRegsetMaskAll = -1,
+ };
struct GPR {
// note: gpr[0] is pc, not x0
@@ -41,7 +43,11 @@ class RegisterInfoPOSIX_riscv64
};
RegisterInfoPOSIX_riscv64(const lldb_private::ArchSpec &target_arch,
- lldb_private::Flags flags);
+ lldb_private::Flags opt_regsets);
+
+ void AddRegSetGP();
+
+ void AddRegSetFP();
size_t GetGPRSize() const override;
@@ -58,9 +64,23 @@ class RegisterInfoPOSIX_riscv64
size_t GetRegisterSetFromRegisterIndex(uint32_t reg_index) const override;
+ bool IsFPPresent() const { return m_opt_regsets.AnySet(eRegsetMaskFP); }
+
+ bool IsFPReg(unsigned reg) const;
+
private:
- const lldb_private::RegisterInfo *m_register_info_p;
- uint32_t m_register_info_count;
+ std::vector<lldb_private::RegisterInfo> m_register_infos;
+
+ std::vector<lldb_private::RegisterSet> m_register_sets;
+
+ // Contains pair of [start, end] register numbers of a register set with start
+ // and end included.
+ std::map<uint32_t, std::pair<uint32_t, uint32_t>> m_per_regset_regnum_range;
+
+ // Register collections to be stored as reference for m_register_sets items
+ std::vector<uint32_t> m_fp_regnum_collection;
+
+ lldb_private::Flags m_opt_regsets;
};
#endif
diff --git a/lldb/source/Plugins/Process/Utility/RegisterInfos_riscv64.h b/lldb/source/Plugins/Process/Utility/RegisterInfos_riscv64.h
index 720d900c7b97e7..be337feabc6440 100644
--- a/lldb/source/Plugins/Process/Utility/RegisterInfos_riscv64.h
+++ b/lldb/source/Plugins/Process/Utility/RegisterInfos_riscv64.h
@@ -79,7 +79,7 @@ using namespace riscv_dwarf;
// clang-format on
-static lldb_private::RegisterInfo g_register_infos_riscv64_le[] = {
+static lldb_private::RegisterInfo g_register_infos_riscv64_gpr[] = {
// DEFINE_GPR64(name, GENERIC KIND)
DEFINE_GPR64(pc, LLDB_REGNUM_GENERIC_PC),
DEFINE_GPR64_ALT(ra, x1, LLDB_REGNUM_GENERIC_RA),
@@ -114,7 +114,9 @@ static lldb_private::RegisterInfo g_register_infos_riscv64_le[] = {
DEFINE_GPR64_ALT(t5, x30, LLDB_INVALID_REGNUM),
DEFINE_GPR64_ALT(t6, x31, LLDB_INVALID_REGNUM),
DEFINE_GPR64_ALT(zero, x0, LLDB_INVALID_REGNUM),
+};
+static lldb_private::RegisterInfo g_register_infos_riscv64_fpr[] = {
DEFINE_FPR64_ALT(ft0, f0, LLDB_INVALID_REGNUM),
DEFINE_FPR64_ALT(ft1, f1, LLDB_INVALID_REGNUM),
DEFINE_FPR64_ALT(ft2, f2, LLDB_INVALID_REGNUM),
@@ -148,7 +150,9 @@ static lldb_private::RegisterInfo g_register_infos_riscv64_le[] = {
DEFINE_FPR64_ALT(ft10, f30, LLDB_INVALID_REGNUM),
DEFINE_FPR64_ALT(ft11, f31, LLDB_INVALID_REGNUM),
DEFINE_FPR_ALT(fcsr, nullptr, 4, LLDB_INVALID_REGNUM),
+};
+static lldb_private::RegisterInfo g_register_infos_riscv64_vpr[] = {
DEFINE_VPR(v0, LLDB_INVALID_REGNUM),
DEFINE_VPR(v1, LLDB_INVALID_REGNUM),
DEFINE_VPR(v2, LLDB_INVALID_REGNUM),
diff --git a/lldb/source/Plugins/Process/elf-core/RegisterContextPOSIXCore_riscv64.cpp b/lldb/source/Plugins/Process/elf-core/RegisterContextPOSIXCore_riscv64.cpp
index 5ba18cdb9889a5..3dca4b16099056 100644
--- a/lldb/source/Plugins/Process/elf-core/RegisterContextPOSIXCore_riscv64.cpp
+++ b/lldb/source/Plugins/Process/elf-core/RegisterContextPOSIXCore_riscv64.cpp
@@ -16,9 +16,17 @@ std::unique_ptr<RegisterContextCorePOSIX_riscv64>
RegisterContextCorePOSIX_riscv64::Create(Thread &thread, const ArchSpec &arch,
const DataExtractor &gpregset,
llvm::ArrayRef<CoreNote> notes) {
+ Flags opt_regsets = RegisterInfoPOSIX_riscv64::eRegsetMaskDefault;
+
+ DataExtractor fpregset = getRegset(notes, arch.GetTriple(), FPR_Desc);
+ if (fpregset.GetByteSize() >= sizeof(uint64_t)) {
+ opt_regsets.Set(RegisterInfoPOSIX_riscv64::eRegsetMaskFP);
+ }
+
return std::unique_ptr<RegisterContextCorePOSIX_riscv64>(
new RegisterContextCorePOSIX_riscv64(
- thread, std::make_unique<RegisterInfoPOSIX_riscv64>(arch, Flags()),
+ thread,
+ std::make_unique<RegisterInfoPOSIX_riscv64>(arch, opt_regsets),
gpregset, notes));
}
@@ -27,17 +35,14 @@ RegisterContextCorePOSIX_riscv64::RegisterContextCorePOSIX_riscv64(
const DataExtractor &gpregset, llvm::ArrayRef<CoreNote> notes)
: RegisterContextPOSIX_riscv64(thread, std::move(register_info)) {
- m_gpr_buffer = std::make_shared<DataBufferHeap>(gpregset.GetDataStart(),
- gpregset.GetByteSize());
- m_gpr.SetData(m_gpr_buffer);
+ m_gpr.SetData(std::make_shared<DataBufferHeap>(gpregset.GetDataStart(),
+ gpregset.GetByteSize()));
m_gpr.SetByteOrder(gpregset.GetByteOrder());
- ArchSpec arch = m_register_info_up->GetTargetArchitecture();
- DataExtractor fpregset = getRegset(notes, arch.GetTriple(), FPR_Desc);
- m_fpr_buffer = std::make_shared<DataBufferHeap>(fpregset.GetDataStart(),
- fpregset.GetByteSize());
- m_fpr.SetData(m_fpr_buffer);
- m_fpr.SetByteOrder(fpregset.GetByteOrder());
+ if (m_register_info_up->IsFPPresent()) {
+ ArchSpec arch = m_register_info_up->GetTargetArchitecture();
+ m_fpr = getRegset(notes, arch.GetTriple(), FPR_Desc);
+ }
}
RegisterContextCorePOSIX_riscv64::~RegisterContextCorePOSIX_riscv64() = default;
diff --git a/lldb/source/Plugins/Process/elf-core/RegisterContextPOSIXCore_riscv64.h b/lldb/source/Plugins/Process/elf-core/RegisterContextPOSIXCore_riscv64.h
index 3cf9531df2c1d2..a9a59844635749 100644
--- a/l...
[truncated]
|
|
The change - is a result of #93297 discussions about FP-register class refactoring |
|
✅ With the latest revision this PR passed the C/C++ code formatter. |
|
Can you remind me what vpr is as opposed to fpr, is that V as in Vector? |
There was a problem hiding this comment.
Using the "dynamic" (as aarch64 calls it) regset idea is certainly the way to go. AArch64 has a bit of a strange setup where we have 2 fixed layouts, then everything else is optional. Possibly it needs to be this way because of SVE changing length, I never dug into it. These days I just use the optional mechanisms for everything.
|
Also consider adding "for riscv64" to the PR title, so it is clear to readers that 32 bit is not included. |
|
Why is this riscv64-only? |
|
Just from lldb's side, the code is organised such that we end up with separate code for 32 and 64 bit in a lot of cases. So a PR for 64 bit only is fine (in fact preferable, as this PR will provide a template for anyone wanting to implement 32 bit support). Of course it's up to @AlexeyMerzlyakov what their plans are here. |
|
@DavidSpickett, thank you for the review and sorry for late reply, I've just returned from the BT w/o an access to my main PC. Yes, the way to handle the register sets is pretty similar that ARM64 is using in dynamic model (if I understand it correctly), so all further added register sets could use this as a baseline, I believe.
Yes, everything is all right. The main thing here - is that some RISC-V platforms might have only general-purpose and vector registers, but non of floating-point ones (e.g. for the case of targets say specifically shaped for ML/AI). For this case, VPR will be placed in totaling structure right after GPR-s. Here it is important to fill registers arrays and numbers (offsets) correctly.
Unfortunately, I have no plans on RV32-s right now. However, the 32-bit support could be added as a set of separate plugins for this target architecture using RV64 as a reference. |
AlexeyMerzlyakov
changed the title
I had not considered that. I'm used to Arm where if you want to have the new vector extensions you have to have the original FP (Neon) extension too. Even more reason to make it all dynamic. |
AlexeyMerzlyakov
force-pushed
the
lldb_fp_reg_opt
branch
from
1a8f739 to
e656524
Compare
|
✅ With the latest revision this PR passed the Python code formatter. |
|
Force-pushed the branch to resolve merge-conflicts after no-update for a some time |
|
Just one more thing then this is good to go. Do you have commit access or will you need me to merge it for you? |
No, I have not. Could you please proceed this? Thanks |
|
Hi @AlexeyMerzlyakov I am looking at commits that warrant a release note for LLDB 20, and I think this is one of them. If you want to do that yourself you can send a PR adding a bullet point to https://github.com/llvm/llvm-project/blob/main/llvm/docs/ReleaseNotes.md#changes-to-lldb, describing the change. If you do not want to, or do not have the time to, I can write it myself. If I do not hear anything from you, I'll assume that I can go ahead with that. |
|
Hi, @DavidSpickett. Sure, no problem, I'll make an update PR for ReleaseNotes with optionally enabled/disabled register sets soon. |
DavidSpickett
added a commit
that referenced
this pull request
Jan 22, 2025
…64 (#123363) This PR adds the release note point for LLDB 20, discussed in #104547 (comment) for the same ticket --------- Co-authored-by: David Spickett <[email protected]>
github-actions bot
pushed a commit
to arm/arm-toolchain
that referenced
this pull request
Jan 22, 2025
…sets for RV64 (#123363) This PR adds the release note point for LLDB 20, discussed in llvm/llvm-project#104547 (comment) for the same ticket --------- Co-authored-by: David Spickett <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The PR adds the support optionally enabled/disabled FP-registers to LLDB
RegisterInfoPOSIX_riscv64. This situation might take place for RISC-V builds having no FP-registers, like RV64IMAC or RV64IMACV.To aim this, patch adds
opt_regsetsflags mechanism. It re-works RegisterInfo class to work with flexibly allocated (depending onopt_regsetsflag)m_register_setsandm_register_infosvectors instead of statically defined structures. The registration of regsets is being arranged bym_per_regset_regnum_rangemap.The patch flows are spread to
NativeRegisterContextLinux_riscv64andRegisterContextCorePOSIX_riscv64classes, that were tested on:EmulateInstructionRISCVis out of scope of this patch, and its behavior did not change, using maximum set of registers.According testcase built for RV64IMAC (no-FPR) was added to
TestLinuxCore.py.