[Lldb-commits] [PATCH] D110569: [lldb] [Process/FreeBSD] Rework arm64 register access

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

mgorny requested changes to this revision.
mgorny added inline comments.
This revision now requires changes to proceed.


================
Comment at: lldb/source/Plugins/Process/FreeBSD/NativeRegisterContextFreeBSD_arm64.cpp:73
 
-Status NativeRegisterContextFreeBSD_arm64::ReadRegisterSet(uint32_t set) {
-  switch (set) {
-  case RegisterInfoPOSIX_arm64::GPRegSet:
-    return NativeProcessFreeBSD::PtraceWrapper(PT_GETREGS, m_thread.GetID(),
-                                               m_reg_data.data());
-  case RegisterInfoPOSIX_arm64::FPRegSet:
-    return NativeProcessFreeBSD::PtraceWrapper(
-        PT_GETFPREGS, m_thread.GetID(),
-        m_reg_data.data() + sizeof(RegisterInfoPOSIX_arm64::GPR));
-  }
-  llvm_unreachable("NativeRegisterContextFreeBSD_arm64::ReadRegisterSet");
+bool NativeRegisterContextFreeBSD_arm64::IsGPR(unsigned reg) const {
+  if (GetRegisterInfo().GetRegisterSetFromRegisterIndex(reg) ==
----------------
These helpers don't seem very helpful. Isn't it better to use `switch` on the value returned by `GetRegisterInfo().GetRegisterSetFromRegisterIndex(reg)`?


================
Comment at: lldb/source/Plugins/Process/FreeBSD/NativeRegisterContextFreeBSD_arm64.cpp:133
+  if (IsGPR(reg)) {
+    error = ReadGPR();
+    if (error.Fail())
----------------
To be honest, it seems counterproductive to restore the duplication that I've removed before. Is there any real advantage to having two methods here, and calling them separately from inside the `if` instead of calling `ReadRegisterSet(set)` before the `if`?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D110569