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

Wed Nov 4 05:38:37 PST 2020

labath added a comment.

[ I've replying to your message in a slightly different order that what it was written. ]

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

> In D82863#2373276 <https://reviews.llvm.org/D82863#2373276>, @labath wrote:
>
>> I'd like to avoid sending bogus offsets, if possible. I'd rather that we teach the server to not send them, and then have the client use this as a cue that it needs to recompute something.
>
> I dont want to write -1 into offset field on server side because it is good helper for accessing data in register buffers but we can introduce a mechanism for all architectures supporting g/G packet we should not send the offset field in qRegisterInfo or target xml. The offset will be constructed by client itself and populated on client side register infos list for helping out with register data management.

Ok, I see where you're going now. If you can pull that off, I think that'd be great. Is that what the current patch does (I haven't looked at the today's update yet)?

Just in case that doesn't work for any reason, I'll elaborate on the idea I had in mind.

>> Then qRegisterInfo (and target.xml) could just check for the -1 value and use that as a cue to skip sending the offset field? All current architectures always hard-code an offset, so this would be a no-op for them...
>
> byte_offset field in RegisterInfo struct is also used elsewhere in code specially to copy data to-from cached register values in data heap buffer. Also on the server side this byte_offset field is used to calculate position of register data in ptrace buffer etc.

The register context could keep relying on the byte_offset for all non-SVE registers. The SVE registers are pretty special anyway, so I was thinking part of their specialness could be that they get their offsets from some other place. Note that we already have the escape hatch called `GetPtraceOffset`, which was added because we could not reconcile the ptrace and `g` uses of the byte_offset fields on x86. So, in general, I think that separating these two things is not a bad direction to go in. For example, the bsd implementations already do a switch over the register number instead of reusing the byte_offset for ptrace purposes. (I think that code could be implemented in a more elegant way, but it is still consistent with that direction.) Your approach also kind of makes that possible (not completely, but it does move us slightly closer towards that), which is why I like it.

>> I'm not sure I understand that. Can you elaborate on the relevance of value_regs here?
>
> So value regs are pseudo register for example V register in Arm64 SVE is a subset of first 16-bytes of a Z register. We do not include value registers in g packet but then need a way to tell where in g/G packet data a specific Vx pseudo register will be located. In my updated patch I update offsets of all primary registers GPR, Z, P, FFR etc and also I copy that offset into their corresponding value registers for example for Z0, same offset is copied to V0, D0, S0. This helps with quick fetching of data on both client and server side.

Ok, I see what you mean. So how will this be handled in your proposal? Will the client "know" that the V/D/S registers are located at particular positions within the Z registers ? Because I think the same solution could be applied to my idea (your proposal is basically a stricter (less flexible) version of what I had in mind -- it does not allow combining registers with offsets and offset-less regs (and that may be more flexibility than we need)).

I think that embedding this kind of knowledge into the client is fine. AFAICT, this is what gdb is doing anyway (I don't see e.g. eax/ax/ah/al being described in the x86_64 target.xml).

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

https://reviews.llvm.org/D82863