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

[Muhammad Omair Javaid via Phabricator via lldb-commits]

omjavaid added a comment.

In D82863#2320342 <https://reviews.llvm.org/D82863#2320342>, @jasonmolenda wrote:

> 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.

I did consider and second the idea of pulling out SVE registers from g/G packet specially for vector length that is more than 8 x (32 bytes) as it creates a huge sized g/G packet containing thousands of bytes. Historically, sending multiple p/P for all registers was an overhead due to slow communication between embedded or serially connected remote targets and host gdb/GDB. This is mostly not the case these days and we can come up with an alternate approach here as being discussed above.

However, There is some catch where we need to preserve all register data across expressions evaluation invocation and the need for offset correctness will still be needed as S,D,V and Z regs share same offset in case of SVE.

Also for futuristic point of view I am baking a patch that adds support for MTE and Pointer Authentication registers. As MT, Pauth and SVE are optional features, therefore registers for these features may or may not be available for a given target and will have to be configured dynamically when inferior is attached. So I was assuming similar adjustment in offsets of MTE/Pauth registers like the ones I have done for SVE registers. However MTE/Pauth registers do not change size dynamically and their offset change will only occur when they are present alongside SVE registers in a particular target.

My idea for all this implementation is to configure an initial offset of all dynamic register register sets at the time of inferior connection in RegisterInfoPOSIX_arm64::ConfigureVectorRegisterInfos where we put SVE register currently right after GPRs (GPR is the bare minimum that all Arm64 targets support) and any dynamically select-able register sets (like MTE, Pauth etc) can come before or after SVE registers.


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

https://reviews.llvm.org/D82863