[lldb] [Target] Make addSupplementaryRegister() work on Register vector

Move DynamicRegisterInfo::AddSupplementaryRegister() into a standalone
function working on std::vector<DynamicRegisterInfo::Register>.

Differential Revision: https://reviews.llvm.org/D111295

lldb/include/lldb/Target/DynamicRegisterInfo.h

@@ -56,11 +56,6 @@ public:
   void AddRegister(lldb_private::RegisterInfo reg_info,
                    lldb_private::ConstString &set_name);
 
-  // Add a new register and cross-link it via invalidate_regs with other
-  // registers sharing its value_regs.
-  void AddSupplementaryRegister(lldb_private::RegisterInfo reg_info,
-                                lldb_private::ConstString &set_name);
-
   void Finalize(const lldb_private::ArchSpec &arch);
 
   size_t GetNumRegisters() const;
@@ -130,6 +125,9 @@ protected:
   bool m_is_reconfigurable = false;
 };
 
+void addSupplementaryRegister(std::vector<DynamicRegisterInfo::Register> &regs,
+                              DynamicRegisterInfo::Register new_reg_info);
+
 } // namespace lldb_private
 
 #endif // LLDB_TARGET_DYNAMICREGISTERINFO_H

lldb/source/Target/DynamicRegisterInfo.cpp

@@ -408,29 +408,6 @@ void DynamicRegisterInfo::AddRegister(RegisterInfo reg_info,
   m_set_reg_nums[set].push_back(reg_num);
 }
 
-void DynamicRegisterInfo::AddSupplementaryRegister(RegisterInfo new_reg_info,
-                                                   ConstString &set_name) {
-  assert(new_reg_info.value_regs != nullptr);
-  const uint32_t reg_num = m_regs.size();
-  AddRegister(new_reg_info, set_name);
-
-  reg_to_regs_map new_invalidates;
-  for (uint32_t value_reg : m_value_regs_map[reg_num]) {
-    // copy value_regs to invalidate_regs
-    new_invalidates[reg_num].push_back(value_reg);
-
-    // copy invalidate_regs from the parent register
-    llvm::append_range(new_invalidates[reg_num], m_invalidate_regs_map[value_reg]);
-
-    // add reverse invalidate entries
-    for (uint32_t x : new_invalidates[reg_num])
-      new_invalidates[x].push_back(new_reg_info.kinds[eRegisterKindLLDB]);
-  }
-
-  for (const auto &x : new_invalidates)
-    llvm::append_range(m_invalidate_regs_map[x.first], x.second);
-}
-
 void DynamicRegisterInfo::Finalize(const ArchSpec &arch) {
   if (m_finalized)
     return;
@@ -802,3 +779,28 @@ DynamicRegisterInfo::GetRegisterInfo(llvm::StringRef reg_name) const {
       return &reg_info;
   return nullptr;
 }
+
+void lldb_private::addSupplementaryRegister(
+    std::vector<DynamicRegisterInfo::Register> &regs,
+    DynamicRegisterInfo::Register new_reg_info) {
+  assert(!new_reg_info.value_regs.empty());
+  const uint32_t reg_num = regs.size();
+  regs.push_back(new_reg_info);
+
+  std::map<uint32_t, std::vector<uint32_t>> new_invalidates;
+  for (uint32_t value_reg : new_reg_info.value_regs) {
+    // copy value_regs to invalidate_regs
+    new_invalidates[reg_num].push_back(value_reg);
+
+    // copy invalidate_regs from the parent register
+    llvm::append_range(new_invalidates[reg_num],
+                       regs[value_reg].invalidate_regs);
+
+    // add reverse invalidate entries
+    for (uint32_t x : new_invalidates[reg_num])
+      new_invalidates[x].push_back(reg_num);
+  }
+
+  for (const auto &x : new_invalidates)
+    llvm::append_range(regs[x.first].invalidate_regs, x.second);
+}

lldb/unittests/Target/DynamicRegisterInfoTest.cpp

@@ -12,6 +12,8 @@
 #include "lldb/Target/DynamicRegisterInfo.h"
 #include "lldb/Utility/ArchSpec.h"
 
+#include <functional>
+
 using namespace lldb_private;
 
 static std::vector<uint32_t> regs_to_vector(uint32_t *regs) {
@@ -69,7 +71,8 @@ protected:
   }
 };
 
-#define ASSERT_REG(reg, ...) { \
+#define ASSERT_REG(reg, ...)                                                   \
+  {                                                                            \
     SCOPED_TRACE("at register " #reg);                                         \
     AssertRegisterInfo(reg, #reg, __VA_ARGS__);                                \
   }
@@ -124,46 +127,62 @@ TEST_F(DynamicRegisterInfoTest, no_finalize_regs) {
   ASSERT_REG(i2, LLDB_INVALID_INDEX32);
 }
 
-TEST_F(DynamicRegisterInfoTest, add_supplementary_register) {
+class DynamicRegisterInfoRegisterTest : public ::testing::Test {
+protected:
+  std::vector<DynamicRegisterInfo::Register> m_regs;
+
+  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};
+    adder(new_reg);
+    return m_regs.size() - 1;
+  }
+
+  void ExpectInRegs(uint32_t reg_num, const char *reg_name,
+                    std::vector<uint32_t> value_regs,
+                    std::vector<uint32_t> invalidate_regs) {
+    ASSERT_GT(m_regs.size(), reg_num);
+
+    const DynamicRegisterInfo::Register &reg = m_regs[reg_num];
+    ConstString expected_reg_name{reg_name};
+    EXPECT_EQ(reg.name, expected_reg_name);
+    EXPECT_EQ(reg.value_regs, value_regs);
+    EXPECT_EQ(reg.invalidate_regs, invalidate_regs);
+  }
+};
+
+#define EXPECT_IN_REGS(reg, ...)                                               \
+  {                                                                            \
+    SCOPED_TRACE("at register " #reg);                                         \
+    ExpectInRegs(reg, #reg, __VA_ARGS__);                                      \
+  }
+
+TEST_F(DynamicRegisterInfoRegisterTest, addSupplementaryRegister) {
   // Add a base register
-  uint32_t rax = AddTestRegister("rax", 8);
+  uint32_t rax = AddTestRegister(
+      "rax", "group", 8,
+      [this](const DynamicRegisterInfo::Register &r) { m_regs.push_back(r); });
 
-  // Register numbers
+  // Add supplementary registers
-  uint32_t eax = 1;
+  auto suppl_adder = [this](const DynamicRegisterInfo::Register &r) {
-  uint32_t ax = 2;
+    addSupplementaryRegister(m_regs, r);
-  uint32_t al = 3;
-
-  ConstString group{"supplementary registers"};
-  uint32_t value_regs[2] = {rax, LLDB_INVALID_REGNUM};
-  struct RegisterInfo eax_reg {
-    "eax", nullptr, 4, LLDB_INVALID_INDEX32, lldb::eEncodingUint,
-        lldb::eFormatUnsigned,
-        {LLDB_INVALID_REGNUM, LLDB_INVALID_REGNUM, LLDB_INVALID_REGNUM, eax,
-         eax},
-        value_regs, nullptr
   };
-  info.AddSupplementaryRegister(eax_reg, group);
+  uint32_t eax = AddTestRegister("eax", "supplementary", 4, suppl_adder, {rax});
+  uint32_t ax = AddTestRegister("ax", "supplementary", 2, suppl_adder, {rax});
+  uint32_t al = AddTestRegister("al", "supplementary", 1, suppl_adder, {rax});
 
-  struct RegisterInfo ax_reg {
+  EXPECT_IN_REGS(rax, {}, {eax, ax, al});
-    "ax", nullptr, 2, LLDB_INVALID_INDEX32, lldb::eEncodingUint,
+  EXPECT_IN_REGS(eax, {rax}, {rax, ax, al});
-        lldb::eFormatUnsigned,
+  EXPECT_IN_REGS(ax, {rax}, {rax, eax, al});
-        {LLDB_INVALID_REGNUM, LLDB_INVALID_REGNUM, LLDB_INVALID_REGNUM, ax, ax},
+  EXPECT_IN_REGS(al, {rax}, {rax, eax, ax});
-        value_regs, nullptr
-  };
-  info.AddSupplementaryRegister(ax_reg, group);
-
-  struct RegisterInfo al_reg {
-    "al", nullptr, 1, LLDB_INVALID_INDEX32, lldb::eEncodingUint,
-        lldb::eFormatUnsigned,
-        {LLDB_INVALID_REGNUM, LLDB_INVALID_REGNUM, LLDB_INVALID_REGNUM, al, al},
-        value_regs, nullptr
-  };
-  info.AddSupplementaryRegister(al_reg, group);
-
-  info.Finalize(lldb_private::ArchSpec());
-
-  ASSERT_REG(rax, 0, {}, {eax, ax, al});
-  ASSERT_REG(eax, 0, {rax}, {rax, ax, al});
-  ASSERT_REG(ax, 0, {rax}, {rax, eax, al});
-  ASSERT_REG(al, 0, {rax}, {rax, eax, ax});
 }