[Lldb-commits] [PATCH] D38323: Enable breakpoints and read/write GPRs for ppc64le

[Greg Clayton via Phabricator via lldb-commits]

clayborg added a comment.

Looks fine. One main questions for new linux archs in particular: is linux using the lldb-server to debug these days even when debugging locally? If so, then this patch only needs to implement both a native register content and not the lldb_private::RegisterContext subclass.



================
Comment at: source/Plugins/Process/Linux/NativeRegisterContextLinux_ppc64le.h:57-64
+  struct RegInfo {
+    uint32_t num_registers;
+    uint32_t num_gpr_registers;
+
+    uint32_t last_gpr;
+
+    uint32_t gpr_flags;
----------------
Any reason for this struct? Just make each member a member variable of the containing class? Is this passed an API somewhere and thus needed?


================
Comment at: source/Plugins/Process/Linux/NativeRegisterContextLinux_ppc64le.h:66-68
+  struct Reg {
+    uint8_t bytes[8];
+  };
----------------
Any reason to not use "uint64_t" instead of "struct Reg"?


================
Comment at: source/Plugins/Process/Utility/RegisterContextPOSIX_ppc64le.h:55-61
+  struct RegInfo {
+    uint32_t num_registers;
+    uint32_t num_gpr_registers;
+
+    uint32_t last_gpr;
+
+    uint32_t gpr_flags;
----------------
Any reason for this struct? Just members of the class?


================
Comment at: source/Plugins/Process/Utility/RegisterContextPOSIX_ppc64le.h:64-66
+  struct Reg {
+    uint8_t bytes[8];
+  };
----------------
uint64_t instead?


https://reviews.llvm.org/D38323