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

[Jason Molenda via Phabricator via lldb-commits]

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