[Lldb-commits] [lldb] 6606327 - [lldb] [DynamicRegisterInfo] Support setting from vector<Register>
Michał Górny via lldb-commits
lldb-commits at lists.llvm.org
Mon Oct 11 08:03:31 PDT 2021
Author: Michał Górny
Date: 2021-10-11T17:02:27+02:00
New Revision: 660632778f308e101c5c168912ed423934aa8b21
URL: https://github.com/llvm/llvm-project/commit/660632778f308e101c5c168912ed423934aa8b21
DIFF: https://github.com/llvm/llvm-project/commit/660632778f308e101c5c168912ed423934aa8b21.diff
LOG: [lldb] [DynamicRegisterInfo] Support setting from vector<Register>
Add an overload of DynamicRegisterInfo::SetRegisterInfo() that accepts
a std::vector<Register> as an argument. This moves the conversion
from DRI::Register to RegisterInfo directly into DynamicRegisterInfo,
and avoids the necessity of creating fully-compatible intermediate
RegisterInfo instances.
While the new method could technically reuse AddRegister(), the ultimate
goal is to replace AddRegister() with SetRegisterInfo() entirely.
Differential Revision: https://reviews.llvm.org/D111435
Added:
Modified:
lldb/include/lldb/Target/DynamicRegisterInfo.h
lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
lldb/source/Target/DynamicRegisterInfo.cpp
lldb/unittests/Target/DynamicRegisterInfoTest.cpp
Removed:
################################################################################
diff --git a/lldb/include/lldb/Target/DynamicRegisterInfo.h b/lldb/include/lldb/Target/DynamicRegisterInfo.h
index bb0b8ec19e5cd..df4c1a64cb339 100644
--- a/lldb/include/lldb/Target/DynamicRegisterInfo.h
+++ b/lldb/include/lldb/Target/DynamicRegisterInfo.h
@@ -53,6 +53,9 @@ class DynamicRegisterInfo {
size_t SetRegisterInfo(const lldb_private::StructuredData::Dictionary &dict,
const lldb_private::ArchSpec &arch);
+ size_t SetRegisterInfo(std::vector<Register> &®s,
+ const lldb_private::ArchSpec &arch);
+
void AddRegister(lldb_private::RegisterInfo reg_info,
lldb_private::ConstString &set_name);
@@ -68,7 +71,7 @@ class DynamicRegisterInfo {
const lldb_private::RegisterSet *GetRegisterSet(uint32_t i) const;
- uint32_t GetRegisterSetIndexByName(lldb_private::ConstString &set_name,
+ uint32_t GetRegisterSetIndexByName(const lldb_private::ConstString &set_name,
bool can_create);
uint32_t ConvertRegisterKindToRegisterNumber(uint32_t kind,
diff --git a/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp b/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
index 80c3b957a16e7..0e77ab01d0bd0 100644
--- a/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
+++ b/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
@@ -4503,32 +4503,7 @@ void ProcessGDBRemote::AddRemoteRegisters(
if (ABISP abi_sp = ABI::FindPlugin(shared_from_this(), arch_to_use))
abi_sp->AugmentRegisterInfo(registers);
- for (auto it : llvm::enumerate(registers)) {
- uint32_t local_regnum = it.index();
- DynamicRegisterInfo::Register &remote_reg_info = it.value();
-
- auto regs_with_sentinel = [](std::vector<uint32_t> &vec) -> uint32_t * {
- if (!vec.empty()) {
- vec.push_back(LLDB_INVALID_REGNUM);
- return vec.data();
- }
- return nullptr;
- };
-
- struct RegisterInfo reg_info {
- remote_reg_info.name.AsCString(), remote_reg_info.alt_name.AsCString(),
- remote_reg_info.byte_size, remote_reg_info.byte_offset,
- remote_reg_info.encoding, remote_reg_info.format,
- {remote_reg_info.regnum_ehframe, remote_reg_info.regnum_dwarf,
- remote_reg_info.regnum_generic, remote_reg_info.regnum_remote,
- local_regnum},
- regs_with_sentinel(remote_reg_info.value_regs),
- regs_with_sentinel(remote_reg_info.invalidate_regs),
- };
- m_register_info_sp->AddRegister(reg_info, remote_reg_info.set_name);
- };
-
- m_register_info_sp->Finalize(arch_to_use);
+ m_register_info_sp->SetRegisterInfo(std::move(registers), arch_to_use);
}
// query the target of gdb-remote for extended target information returns
diff --git a/lldb/source/Target/DynamicRegisterInfo.cpp b/lldb/source/Target/DynamicRegisterInfo.cpp
index 7f8394d1ac849..bd56c239a574c 100644
--- a/lldb/source/Target/DynamicRegisterInfo.cpp
+++ b/lldb/source/Target/DynamicRegisterInfo.cpp
@@ -379,6 +379,45 @@ DynamicRegisterInfo::SetRegisterInfo(const StructuredData::Dictionary &dict,
return m_regs.size();
}
+size_t DynamicRegisterInfo::SetRegisterInfo(
+ std::vector<DynamicRegisterInfo::Register> &®s,
+ const ArchSpec &arch) {
+ assert(!m_finalized);
+
+ for (auto it : llvm::enumerate(regs)) {
+ uint32_t local_regnum = it.index();
+ const DynamicRegisterInfo::Register ® = it.value();
+
+ assert(reg.name);
+ assert(reg.set_name);
+
+ if (!reg.value_regs.empty())
+ m_value_regs_map[local_regnum] = std::move(reg.value_regs);
+ if (!reg.invalidate_regs.empty())
+ m_invalidate_regs_map[local_regnum] = std::move(reg.invalidate_regs);
+
+ struct RegisterInfo reg_info {
+ reg.name.AsCString(), reg.alt_name.AsCString(), reg.byte_size,
+ reg.byte_offset, reg.encoding, reg.format,
+ {reg.regnum_ehframe, reg.regnum_dwarf, reg.regnum_generic,
+ reg.regnum_remote, local_regnum},
+ // value_regs and invalidate_regs are filled by Finalize()
+ nullptr, nullptr
+ };
+
+ m_regs.push_back(reg_info);
+
+ uint32_t set = GetRegisterSetIndexByName(reg.set_name, true);
+ assert(set < m_sets.size());
+ assert(set < m_set_reg_nums.size());
+ assert(set < m_set_names.size());
+ m_set_reg_nums[set].push_back(local_regnum);
+ };
+
+ Finalize(arch);
+ return m_regs.size();
+}
+
void DynamicRegisterInfo::AddRegister(RegisterInfo reg_info,
ConstString &set_name) {
assert(!m_finalized);
@@ -682,8 +721,9 @@ const RegisterSet *DynamicRegisterInfo::GetRegisterSet(uint32_t i) const {
return nullptr;
}
-uint32_t DynamicRegisterInfo::GetRegisterSetIndexByName(ConstString &set_name,
- bool can_create) {
+uint32_t
+DynamicRegisterInfo::GetRegisterSetIndexByName(const ConstString &set_name,
+ bool can_create) {
name_collection::iterator pos, end = m_set_names.end();
for (pos = m_set_names.begin(); pos != end; ++pos) {
if (*pos == set_name)
diff --git a/lldb/unittests/Target/DynamicRegisterInfoTest.cpp b/lldb/unittests/Target/DynamicRegisterInfoTest.cpp
index 04bd7e67e3238..94c533721ab9f 100644
--- a/lldb/unittests/Target/DynamicRegisterInfoTest.cpp
+++ b/lldb/unittests/Target/DynamicRegisterInfoTest.cpp
@@ -130,20 +130,26 @@ TEST_F(DynamicRegisterInfoTest, no_finalize_regs) {
class DynamicRegisterInfoRegisterTest : public ::testing::Test {
protected:
std::vector<DynamicRegisterInfo::Register> m_regs;
+ DynamicRegisterInfo m_dyninfo;
uint32_t AddTestRegister(
const char *name, const char *group, uint32_t byte_size,
std::function<void(const DynamicRegisterInfo::Register &)> adder,
std::vector<uint32_t> value_regs = {},
std::vector<uint32_t> invalidate_regs = {}) {
- DynamicRegisterInfo::Register new_reg{
- ConstString(name), ConstString(),
- ConstString(group), byte_size,
- LLDB_INVALID_INDEX32, lldb::eEncodingUint,
- lldb::eFormatUnsigned, LLDB_INVALID_REGNUM,
- LLDB_INVALID_REGNUM, LLDB_INVALID_REGNUM,
- LLDB_INVALID_REGNUM, value_regs,
- invalidate_regs};
+ DynamicRegisterInfo::Register new_reg{ConstString(name),
+ ConstString(),
+ ConstString(group),
+ byte_size,
+ LLDB_INVALID_INDEX32,
+ lldb::eEncodingUint,
+ lldb::eFormatUnsigned,
+ LLDB_INVALID_REGNUM,
+ LLDB_INVALID_REGNUM,
+ LLDB_INVALID_REGNUM,
+ static_cast<uint32_t>(m_regs.size()),
+ value_regs,
+ invalidate_regs};
adder(new_reg);
return m_regs.size() - 1;
}
@@ -159,6 +165,18 @@ class DynamicRegisterInfoRegisterTest : public ::testing::Test {
EXPECT_EQ(reg.value_regs, value_regs);
EXPECT_EQ(reg.invalidate_regs, invalidate_regs);
}
+
+ void ExpectInDynInfo(uint32_t reg_num, const char *reg_name,
+ uint32_t byte_offset,
+ std::vector<uint32_t> value_regs = {},
+ std::vector<uint32_t> invalidate_regs = {}) {
+ const RegisterInfo *reg = m_dyninfo.GetRegisterInfoAtIndex(reg_num);
+ ASSERT_NE(reg, nullptr);
+ EXPECT_STREQ(reg->name, reg_name);
+ EXPECT_EQ(reg->byte_offset, byte_offset);
+ EXPECT_THAT(regs_to_vector(reg->value_regs), value_regs);
+ EXPECT_THAT(regs_to_vector(reg->invalidate_regs), invalidate_regs);
+ }
};
#define EXPECT_IN_REGS(reg, ...) \
@@ -167,6 +185,12 @@ class DynamicRegisterInfoRegisterTest : public ::testing::Test {
ExpectInRegs(reg, #reg, __VA_ARGS__); \
}
+#define EXPECT_IN_DYNINFO(reg, ...) \
+ { \
+ SCOPED_TRACE("at register " #reg); \
+ ExpectInDynInfo(reg, #reg, __VA_ARGS__); \
+ }
+
TEST_F(DynamicRegisterInfoRegisterTest, addSupplementaryRegister) {
// Add a base register
uint32_t rax = AddTestRegister(
@@ -186,3 +210,32 @@ TEST_F(DynamicRegisterInfoRegisterTest, addSupplementaryRegister) {
EXPECT_IN_REGS(ax, {rax}, {rax, eax, al});
EXPECT_IN_REGS(al, {rax}, {rax, eax, ax});
}
+
+TEST_F(DynamicRegisterInfoRegisterTest, SetRegisterInfo) {
+ auto adder = [this](const DynamicRegisterInfo::Register &r) {
+ m_regs.push_back(r);
+ };
+ // Add regular registers
+ uint32_t b1 = AddTestRegister("b1", "base", 8, adder);
+ uint32_t b2 = AddTestRegister("b2", "other", 8, adder);
+
+ // Add a few sub-registers
+ uint32_t s1 = AddTestRegister("s1", "base", 4, adder, {b1});
+ uint32_t s2 = AddTestRegister("s2", "other", 4, adder, {b2});
+
+ // Add a register with invalidate_regs
+ uint32_t i1 = AddTestRegister("i1", "third", 8, adder, {}, {b1});
+
+ // Add a register with indirect invalidate regs to be expanded
+ // TODO: why is it done conditionally to value_regs?
+ uint32_t i2 = AddTestRegister("i2", "third", 4, adder, {b2}, {i1});
+
+ EXPECT_EQ(m_dyninfo.SetRegisterInfo(std::move(m_regs), ArchSpec()),
+ m_regs.size());
+ EXPECT_IN_DYNINFO(b1, 0, {}, {});
+ EXPECT_IN_DYNINFO(b2, 8, {}, {});
+ EXPECT_IN_DYNINFO(s1, 0, {b1}, {});
+ EXPECT_IN_DYNINFO(s2, 8, {b2}, {});
+ EXPECT_IN_DYNINFO(i1, 16, {}, {b1});
+ EXPECT_IN_DYNINFO(i2, 8, {b2}, {b1, i1});
+}
More information about the lldb-commits
mailing list