[Lldb-commits] [PATCH] D108831: [lldb] [gdb-remote] Add x86_64 pseudo-registers when using gdbserver

[Pavel Labath via Phabricator via lldb-commits]

labath added inline comments.


================
Comment at: lldb/source/Plugins/Process/gdb-remote/GDBRemoteRegisterContext_x86.cpp:37-54
+static std::array<PartialGPRRegSet, 16> partial_gpr_regs = {{
+    {"rax", {{"eax"}, {"ax"}, {"ah"}, {"al"}}},
+    {"rbx", {{"ebx"}, {"bx"}, {"bh"}, {"bl"}}},
+    {"rcx", {{"ecx"}, {"cx"}, {"ch"}, {"cl"}}},
+    {"rdx", {{"edx"}, {"dx"}, {"dh"}, {"dl"}}},
+    {"rdi", {{"edi"}, {"di"}, {nullptr}, {"dil"}}},
+    {"rsi", {{"esi"}, {"si"}, {nullptr}, {"sil"}}},
----------------
mgorny wrote:
> Also here I'm wondering if it wouldn't be better to just have an array of `{"ax", "bx", ...}` and construct all other names from that base, plus r8..r15 via numeric `for` loop.
I don't have a strong opinion on this. The names are sufficiently irregular that the other approach will (probably) be messy as well.

I'm more bothered by the fact that you're writing into these global variables. IIRC, the DynamicRegisterInfo class has some sort of a buffer to store the array data. It would be good to reuse that (or do something similar).


================
Comment at: lldb/source/Plugins/Process/gdb-remote/GDBRemoteRegisterContext_x86.cpp:66-75
+static std::array<MMReg, 16> mm_regs = {{
+    {"st0", "mm0"},
+    {"st1", "mm1"},
+    {"st2", "mm2"},
+    {"st3", "mm3"},
+    {"st4", "mm4"},
+    {"st5", "mm5"},
----------------
mgorny wrote:
> I've been somewhat basing this thing on how our process plugins do registers but I'm wondering if it wouldn't be better to just do a `for (int x = 0; i < 8; i++)` loop and dynamically generate `st${x}` and `mm${x}` names.
The loop idea seems reasonable here.


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

https://reviews.llvm.org/D108831