[Lldb-commits] [PATCH] D91241: [LLDB] Make offset field optional in RegisterInfo packet for Arm64

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

This looks pretty straightforward, modulo the inline comment about the centralization of the offset computations....



================
Comment at: lldb/source/Plugins/Process/Utility/DynamicRegisterInfo.cpp:534
 
+  // On AArch64 architecture we set offsets on client side in accordance with
+  // GDB g/G packet standard in sequence of increasing register numbers.
----------------
This part seems dubious. It would be better if it were not tied to the architecture, but to the fact that the stub did not provide the register offset for these registers. It would also be better if it was placed together with the rest of the code for computing the register offsets.


================
Comment at: lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp:571
+        } else
+          reg_offset += reg_info.byte_size;
+
----------------
Which, I guess, means here.


================
Comment at: lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp:4438
+        } else
+          reg_offset += reg_info.byte_size;
+
----------------
And here (feel free to factor this into some kind of a utility function).


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

https://reviews.llvm.org/D91241