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

Thu Nov 5 00:45:25 PST 2020

omjavaid added a comment.

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

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

GDB does not have pseudo registers as part of xml register description. GDB is managing pseudo registers on client side and independent of rest of the register set. In case of SVE Z registers, GDB describes a composite type kind of a union of (Z, V, D and S) which helps in giving registers a view in terms of V, S and D which are not actually register.

As invalidate_regs and value_regs are totally LLDB specific so we can put information about LLDB speicific pseudo registers in those lists which we actually do by the way. Taking advantage of that I have S, V and D registers in invalidate_reg list of a Z register which share the same starting offset as the corresponding Z register but are size restricted to 4, 8 or 16 bytes.

For the sake of clarity here is the scheme:

We have two types of registers:

1. Primary registers

Includes 64 bit GPRs Xn regs, PC etc
Also includes V registers for all non-sve targets
Includes Z, P, FFR and VG for SVE targets.

All primary registers have value_regs list set to nullptr
All primary registers have invalidate_regs list which is a list of registers which share same starting offset as the corresponding primary registers.

2. Pseudo registers

Includes 32 bit GPRs Wn regs
Includes D and S registers for all non SVE targets
Also includes V, D and S registers for all SVE targets

All pseudo register have a value register which can be found in value_regs list of that register.
All pseudo registers have invalidate_regs list which is a list of registers which share same starting offset as the corresponding primary registers.

On start of debug session we exchange qRegisterInfo or target XML packet registers on client side are numbered in sequence they are received in qRegisterInfo or their placement location in XML description. We receive register offset field populated in case we are talking to lldb-server. This offset will be ignored in case of AArch64 target with SVE and it will be recalculated in DynamicRegisterInfo::Finalize function.

Moreover, whenever we stop we get register VG in expedited registers list. we call GDBRemoteRegisterContext::AArch64SVEReconfigure function which will evaluate if we need to update offsets. In case VG has been updated whole register list will be traversed in sequence and new offsets will be assigned according to updated register sizes. All pseudo registers will share the offset of their primary register.

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

https://reviews.llvm.org/D82863