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

_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Reply via email to