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 _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits