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

Fri Oct 9 06:11:53 PDT 2020

labath added a comment.

In D82863#2321370 <https://reviews.llvm.org/D82863#2321370>, @omjavaid wrote:

> In D82863#2320342 <https://reviews.llvm.org/D82863#2320342>, @jasonmolenda wrote:
>
>> 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. :)

Well.. we could design it such that the SVE registers (and any other dynamically-sized regs) always come last. Then they wouldn't impact the offsets of static registers.

I don't think we have a requirement that newer register sets must be appended. Or at least, we shouldn't have, as now both intel and arm have cpu features (and registers) that may be arbitrarily combined in future processors.

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

While it's certainly less important than it used to be, there are still people who care about these things. Just last year, we had a contribution which started using them more aggressively. The contributor has a use case for fetching all registers on each stop, which meant that our usual expedition rules were not sufficient and p packets were slow.

> and we can come up with an alternate approach here as being discussed above.

That's true, but any custom solution is unlikely to be supported by stubs (qemu) which are primarily meant for communicating with gdb. And since we have people who want to use those stubs with lldb (and I am supporting some of them), if we do something custom, it's possible we may end up supporting both.

That's why I'd like to understand more about how gdb does things. The g packet definitely has its issues but so does introducing custom packets, and I think we should weigh those carefully.

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

Is this about QSave/RestoreAllRegisters? If it is, then I don't think it's that very relevant here, because these saved registers don't make their way across the wire, and the fact that we save the registers in a buffer similar to the g packet is purely an implementation detail. We can change that any time we like and it does not require the client&server to be in sync. (Of course, if we do serialize everything into the g packet, then reusing that format for QSaveAllRegisters is handy, so it is still /slightly/ relavant).

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

https://reviews.llvm.org/D82863