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

[Pavel Labath via Phabricator via lldb-commits]

labath added inline comments.


================
Comment at: lldb/source/Plugins/Process/gdb-remote/GDBRemoteRegisterContext.cpp:792-793
+      }
+      reg.byte_offset = end_reg_offset;
+      end_reg_offset += reg.byte_size;
+    }
----------------
We already have a mostly generic algorithm for computing the initial offsets of registers. This code reintroduces the assumptions about the ordering of registers and similar.

 It would be nice if this could reuse that offset computation code (after updating the register sizes).  One possibility might be to stash the original register infos (with empty byte offsets) somewhere, and then reinitialize this register context from *that*. 


================
Comment at: lldb/source/Plugins/Process/gdb-remote/GDBRemoteRegisterContext.cpp:767
+      m_reg_data.Clear();
+      m_reg_data.SetData(reg_data_sp);
+      m_reg_data.SetByteOrder(GetByteOrder());
----------------
omjavaid wrote:
> jasonmolenda wrote:
> > I'm probably not following this correctly, but isn't this going to shorten the register buffer m_reg_data to the end of the SVE registers in the buffer, right?  If the goal here is to create a heap object, why not just copy the entire m_data_reg?  If someone ever adds a register past the SVE's, this would truncate it away.  
> Register data can expand on shrink based on vector length update of SVE register set. So heap size will change accordingly.
> We need to preserve VG register and GPRs from current register data so we copy the whole data in case vector length update caused register data to expand. However we truncate data to new vector length in case vector length has shrunk. All register past VG registers are invalidated anyways. 
How often do you expect the user to be changing the vector length? Could we just create a completely empty (invalid) buffer of the right size and force a re-fetch the next time the registers are accessed? With `g` packets, this should be a single round-trip anyway...


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

https://reviews.llvm.org/D82863