[Lldb-commits] [PATCH] D90034: [lldb] [Process/NetBSD] Use XStateRegSet for all FPU registers

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

I don't know much about netbd, but it definitely seems cleaner...



================
Comment at: lldb/source/Plugins/Process/NetBSD/NativeRegisterContextNetBSD_x86_64.cpp:150
+// x86 32-bit registers available via XState.
+static const uint32_t g_xstate_regnums_i386[] = {
     lldb_fctrl_i386,    lldb_fstat_i386,     lldb_ftag_i386,  lldb_fop_i386,
----------------
This is in an anonymous namespace, so static is superfluous. However, this anonymous namespace is against the [[ http://llvm.org/docs/CodingStandards.html#anonymous-namespaces | coding standards ]] so you might as well finish adding statics, and remove it. :)


================
Comment at: lldb/source/Plugins/Process/NetBSD/NativeRegisterContextNetBSD_x86_64.cpp:994-995
         !(m_xstate.xs_rfbm & XCR0_YMM_Hi128)) {
-      error.SetErrorStringWithFormat("register \"%s\" not supported by CPU/kernel",
-                                     reg_info->name);
+      error.SetErrorStringWithFormat(
+          "register \"%s\" not supported by CPU/kernel", reg_info->name);
     } else {
----------------
Is this code actually reachable? Ideally, I'd expect this to be checked immediately after program startup, so that we can avoid even reporting the existence of [xy]mm registers..


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

https://reviews.llvm.org/D90034