[Lldb-commits] [PATCH] D82863: [LLDB] Add support to resize SVE registers at run-time

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

Thanks for your patience. I think this is in a pretty good shape now. Just a couple of quick questions inline...



================
Comment at: lldb/source/Plugins/Process/gdb-remote/GDBRemoteRegisterContext.cpp:410
+          if (success && do_reconfigure_arm64_sve &&
+              GetPrimordialRegister(reg_info, gdb_comm))
+            AArch64SVEReconfigure();
----------------
Could we move the `GetPrimordialRegister` part into `AArch64SVEReconfigure`? The function looks up the "vg" number again anyway?

Actually, why is this even needed? Wouldn't the `ReadRegisterAsUnsigned(vg_reg_num)` call inside `AArch64SVEReconfigure` already handle the reading aspect?


================
Comment at: lldb/source/Plugins/Process/gdb-remote/GDBRemoteRegisterContext.cpp:752-753
+      // Make a heap based buffer that is big enough to store all registers
+      DataBufferSP reg_data_sp(
+          new DataBufferHeap(m_reg_info_sp->GetRegisterDataByteSize(), 0));
+
----------------
```
auto reg_data_sp = std::make_shared<DataBufferHeap>(m_reg_info_sp->GetRegisterDataByteSize(), 0);
```
Or maybe even drop the local var and move directly into the `SetData` call.


================
Comment at: lldb/source/Plugins/Process/gdb-remote/GDBRemoteRegisterContext.cpp:755
+
+      m_reg_data.Clear();
+      m_reg_data.SetData(reg_data_sp);
----------------
Is Clear before SetData really needed?


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

https://reviews.llvm.org/D82863