jasonmolenda added a comment.

In D82863#2319568 <https://reviews.llvm.org/D82863#2319568>, @labath wrote:

> In D82863#2313676 <https://reviews.llvm.org/D82863#2313676>, @jasonmolenda 
> wrote:
>
>> (about g packets...) they cause so many problems if there is a 
>> mis-coordination between the remote stub and lldb, I never want to stop the 
>> remote stub from providing those offsets.
>
> I am not sure how to interpret this in the context of SVE registers. The stub 
> cannot send the offsets of those, as their size/offset might change depending 
> on their configuration.

Yeah, of course you're right, in the case of a dynamic register context like 
this, the g/G packets shouldn't be used (we should have 
GDBRemoteCommunicationClient::AvoidGPackets return true).

The jThreadsInfo may give us the full register context (in base10 because json 
lol) although debugserver only sends the GPRs in the jThreadsInfo response.  If 
lldb needs to read the full register context, it would need to read them 
individually (and write them individually if saving the full register context). 
 We could add qReadAllRegisters and QWriteAllRegisters which have a series of 
regnum:base16-regval-target-endian, or JSON versions of the same packets if the 
perf was important.

The original g/G packets were designed for little embedded systems where the 
stub had to be very small and dumb, and could just memcpy the payload in the 
packet into the register context & back out again.  Any sensible design today 
would, at least, have some form of an array of regnum:regval to eliminate many 
of the problems.

> Unless of course, we make sure SVE regs come last, but that imposes some 
> constraints on future registers sets. I suppose we might be able to say that 
> all variable-length or otherwise-funny registers must come last.

Yeah, Omair's patch currently assumes that the SVE regs come last already when 
they copy & truncate the registers context to heap.  I fear that we'll get to 
armv12 and we'll be adding registers after the SVE and wonder why they're being 
truncated somewhere in lldb. :)

@omjavaid , what do you think about disabling g/G when we're working with SVE 
registers (GDBRemoteCommunicationClient::AvoidGPackets)?  There are some 
gdb-remote stubs that can *only* read/write registers with g/G, but I think 
it's reasonable to say "you must implement p/P for a target with SVE", that's a 
generic packet shared by both lldb and gdb.  We could add a more-modern g/G 
replacement packet, but no one would have that implemented, and if they were 
going to add anything, I'd have them implement p/P unless it's perf problems 
and they need a read-all-registers / write-all-registers packet that works with 
SVE.


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