[Lldb-commits] [PATCH] D110023: [lldb] [DynamicRegisterInfo] Add a convenience method to add suppl. registers

[Michał Górny via Phabricator via lldb-commits]

mgorny marked 4 inline comments as done.
mgorny added inline comments.


================
Comment at: lldb/source/Plugins/Process/Utility/DynamicRegisterInfo.cpp:441
+
+  reg_to_regs_map to_add;
+  for (uint32_t value_reg : m_value_regs_map[reg_num]) {
----------------
labath wrote:
> Maybe call this new_invalidates? I've found it hard to track what to_add means, with all the mixing of value_regs and invalidates...
Good idea.


================
Comment at: lldb/source/Plugins/Process/Utility/DynamicRegisterInfo.cpp:443-444
+  for (uint32_t value_reg : m_value_regs_map[reg_num]) {
+    if (value_reg == LLDB_INVALID_REGNUM)
+      break;
+
----------------
labath wrote:
> Is this still needed?
Probably not indeed.


================
Comment at: lldb/unittests/Process/Utility/DynamicRegisterInfoTest.cpp:184-190
+  struct RegisterInfo ah_reg {
+    "ah", nullptr, 1, LLDB_INVALID_INDEX32, lldb::eEncodingUint,
+        lldb::eFormatUnsigned,
+        {LLDB_INVALID_REGNUM, LLDB_INVALID_REGNUM, LLDB_INVALID_REGNUM, ah, ah},
+        value_regs, nullptr, nullptr, 0
+  };
+  info.AddSupplementaryRegister(ah_reg, group);
----------------
labath wrote:
> Could we remove ah from this test, as its offset is going to be wrong?
Sure.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D110023/new/

https://reviews.llvm.org/D110023