Re: [PATCH v3 0/8] gdbserver improvements for AArch64 SVE support

Thiago Jung Bauermann
2024-09-10T05:36:52Z

I'm finally reviving this work, and will have a talk about it at GNU
Tools Cauldron next Monday. If you are going to be there, we can discuss
in person.

"Willgerodt, Felix" <felix.willgerodt@intel.com> writes:

>> -----Original Message-----
>> From: Thiago Jung Bauermann <thiago.bauermann@linaro.org>
>>
>> Thank you for your interest in this series! I think it's very important
>> too, and it nags me that I haven't been able to finish it yet. I gave up
>> on the approach from this patch series, which was to have a different
>> target description for each register size, which implies a different
>> target description for each inferior thread.
>>
>> I have a branch where I implemented support in the XML target
>> description to specify a register whose size is given by a math
>> expression involving other registers. This way, a single target
>> description is enough for the whole duration of the inferior execution,
>> and for all inferior threads. But it still has significant bugs, and
>> only occasionally I have been able to get back to it and try to move it
>> forward. I pushed what I have to the sve-tdesc-dynamic-size branch at
>> this repo, if you would like to have a look:
>>
>> https://git.linaro.org/people/thiago.bauermann/binutils-gdb.git
>>
>> If you "git diff 1bdabb9e9fe8..<remote>/sve-tdesc-dynamic-size" you can
>> see the code.
>>
>> I think I will be able to work on it again in a month or two. If you
>> think my approach is good and the branch is a good starting point for
>> you, feel free to work on it. I'd be happy to collaborate.
>>
>
> Thanks for the clarification.
> I briefly looked at your branch, but I didn't have enough time to really
> understand it yet. But I still wanted to give some feedback based on
> my current knowledge and what you wrote.

Thank you!

> On x86 we will also need resizable registers. Though on x86, the
> sizing information is only partially found in other registers. Some of
> the sizing information comes e.g. from the CPUID instruction
> (basically an instruction to query features of your CPU, no idea if
> there is an ARM equivalent).

I think this won't be a problem with my current approach. The basic idea
is to encode a simple math expression in the description XML (using
MathML notation). A register value will be referenced with a <ci>
element, which specifies a variable.

You could define a variable name to mean the information from the CPUID
instruction or some other source.

> And x86 would also need the full
> flexibility of adding or removing registers I think. So would the
> Intel GPU port, that is currently still a downstream fork.

I don't address the possibility of adding or removing registers.
I believe my approach doesn't make it easier nor harder to add support
for it though.

> I realize that this is mainly our problem. We will start working on
> This eventually. Though I think any series touching generic code
> should offer enough flexibility for all architectures. I wonder
> if your new approach offers that (I actually don't know),
> if it is only based on other registers and only offers resizing.

I think my new approach adds flexibility in the sense that it allows for
a new way to specify register sizes that is very flexible (simple math
expressions using the contents of other registers, or even other kinds
of information) while not getting in the way of additional approaches
that may be added in the future, including the previous approach of
per-thread target descriptions.

> Though if it doesn't prevent adding additional input for re-sizing
> and could live in parallel to adding/removing registers at runtime
> it could be viewed as ok.

Yes, I believe that is the case. I think it makes easier to add
additional input for resizing, by way of new variables which can be used
in math expressions that represent those inputs. And it could live in
parallel to adding/removing registers at runtime.

> But it makes me wonder a bit if the older approach isn't more
> flexible for future architectures that want to use it. And it also
> seems to be closer to the way GDB has already implemented.
> Of course that doesn't mean we can't switch GDB to
> a better solution at some point.
>
> So I am curious: What was your reason for switching to the
> new approach?

I think Pedro summed up well the main concerns with the per-thread
target description approach for the case of SVE support:

https://inbox.sourceware.org/gdb-patches/045c1e09-5b6d-22b6-df7a-39cfa339b0e1@palves.net/

I agree with him that conceptually SVE would be best modelled in GDB as
a target description feature specifying variable-sized registers. It
isn't like that in the way GDB is currently implemented (and also in
this patch series we're replying to) because of practical concerns with
the complexity of the ideal solution.

There was also concern about adding RSP support for it if it's not the
best approach, because when adding a new feature to the remote protocol
it's not easy to change or drop support for it later.

I had researched into how to implement this new approach before posting
the first version of this patch series, but at the time I wasn't able to
find a way in a reasonable amount of time. After this feedback, I
decided to try again and this time I was able to find a way to implement
it.

That doesn't mean that per-thread target description aren't the best
model for other situations. For instance, if a system has different
processor models in a single core then it is the natural approach.

Also, if in your case registers can be added or removed, maybe it's the
best approach as well.

--
Thiago