Re: [PATCH,v2 17/17] [gdb/docs] sme: Document SME registers and features

[Luis Machado <luis.machado@arm.com>]

Hi Eli,

Thanks for the feedback on the documentation. I have some comments about your suggestion, but I think
we're converging.

On 4/13/23 08:57, Eli Zaretskii wrote:
>> From: Luis Machado <luis.machado@arm.com>
>> Date: Wed, 12 Apr 2023 13:04:44 +0100
>>
>> Updates since v1:
>>
>> - Made SME text more thorough.
>> - Adjusted text based on upstream reviews.
>> - Fixed documentation errors (missing itemization for SME registers).
>>
>> Provide documentation for the SME feature and other information that
>> should be useful for users that need to debug a SME-capable target.
>> ---
>>   gdb/NEWS            |  11 +++
>>   gdb/doc/gdb.texinfo | 223 ++++++++++++++++++++++++++++++++++++++++++++
>>   2 files changed, 234 insertions(+)
> 
> Thanks.
> 
>> diff --git a/gdb/NEWS b/gdb/NEWS
>> index 10a1a70fa52..48a82172f0e 100644
>> --- a/gdb/NEWS
>> +++ b/gdb/NEWS
>> @@ -3,6 +3,17 @@
>>   
>>   *** Changes since GDB 13
>>   
>> +* GDB now supports the AArch64 Scalable Matrix Extension (SME), which includes
>> +  a new matrix register named ZA, a new thread register TPIDR2 and a new vector
>> +  length register SVG (streaming vector granule).  GDB also supports tracking
>> +  ZA state across signal frames.
>> +
>> +  Some features are still under development or are dependent on ABI specs that
>> +  are still in alpha stage.  For example, manual function calls with ZA state
>> +  don't have any special handling, and tracking of SVG changes based on
>> +  DWARF information is still not implemented, but there are plans to do so in
>> +  the future.
> 
> This part is OK.
> 
>> +For SVE, the following definitions are used throughout @value{GDBN}'s source
>> +code and in this document:
>> +
>> +@itemize
>> +
>> +@item
>> +@anchor{VL}
>> +@cindex VL
>> +@code{VL}: The vector length, in bytes.  It defines the size of each @code{Z}
>> +register.
>> +
>> +@item
>> +@anchor{VQ}
>> +@cindex VQ
>> +@code{VQ}: The number of 128 bit units in @code{VL}.  This is mostly used
>> +internally by @value{GDBN} and the Linux Kernel.
>> +
>> +@item
>> +@anchor{VG}
>> +@cindex VG
>> +@code{VG}: The number of 64 bit units in @code{VL}.  This is mostly used
>> +internally by @value{GDBN} and the Linux Kernel.
> 
> I suggest to call these parameters @var{nvl}, @var{nvq}, and
> @var{nvg}, respectively.  That is: (1) lower-case names, (2) name them
> differently from the corresponding register, and (3) use @var markup.
> This could mean the names are no longer identical to what you use in
> the GDB sources, but the text is easier to read and less confusing,
> because, for example, the difference between SVG the parameter and SVG
> the register name is tricky, and confused even you (or at least it
> confused me, see below).
> 

I'm not sure that would be a good idea. These definitions (VL, VG and VQ, as well as the SME counterparts SVL, SVG and SVQ)
are supposed to provide different views into the same data, the SVE vector length or the SME vector length.

They are only supposed to help describe the concept of SVE/SME.

When we're talking about how gdb implements SVE and SME, then we're talking about registers vg and svg, which
map directly to the VG and SVG definitions.

I may be leaking some implementation details to the user's manual. Would it help if we only mention
these definitions when explaining the concept, but then restrict ourselves to only using the
vg and svg registers when explaining how gdb deals with SVE and SME?

But it is convenient, when detailing the sizes of registers, to use SVL, which doesn't have a
direct mapping to a gdb register.

>> +by providing a 2-dimensional square matrix of variable size called @code{ZA},
>> +just like SVE provides a group of vector registers of variable size, the
>> +32 @code{Z} registers.
> 
> ZA is a register, no?  Then I suggest
> 
>    by providing a 2-dimensional register @code{ZA}, which is a square
>    matrix of variable size, just like [...]
> 

Fixed.

>> +The following definitions are used throughout @value{GDBN}'s source code and
>> +in this document:
>> +
>> +@itemize
>> +
>> +@item
>> +@anchor{SVL}
>> +@cindex SVL
>> +@code{SVL}: The streaming vector length, in bytes.  It defines the size of each
>> +dimension of the 2-dimensional square @code{ZA} matrix.  The total size of
>> +@code{ZA} is therefore @code{@var{SVL}x@var{SVL}}.
>> +
>> +@item
>> +@anchor{SVQ}
>> +@cindex SVQ
>> +@code{SVQ}: The number of 128 bit units in @code{SVL}.  This is mostly used
>> +internally by @value{GDBN} and the Linux Kernel.
>> +
>> +@item
>> +@anchor{SVG}
>> +@cindex SVG
>> +@code{SVG}: The number of 64 bit units in @code{SVL}.  This is mostly used
>> +internally by @value{GDBN} and the Linux Kernel.
> 
> Likewise here: I suggest to use @var{nsvl} etc. for these parameters.
> 

Pending on my first point above.

>> +The @code{za} register is a 2-dimensional square @code{@var{n}x@var{n}}
>> +matrix of bytes, where @var{n} is the streaming vector length (@code{SVL}.
> 
> Here you could use the parameter explicitly:
> 
>    The @code{za} register is a 2-dimensional square
>    @code{@var{nsvl}x@var{nsvl}} matrix of bytes.
> 

I'm having a hard time seeing the benefits of renaming SVL/SVQ/SVG to nsvl, nsvq and nsvg.

The definition for SVL/SVQ/SVG is made clear above, at least I see it that way.

My intent is to introduce SME enough so users are aware this feature is supported by gdb and are aware of
how gdb exposes it.

>> +@xref{SVL}
> 
> This reference is unnecessary, as the description is only a short ways
> above, and in the same node.
> 

Removed now.

>> +If the user wants to index the @code{za} register as a matrix, it is possible
>> +to reference @code{za} as @code{za[i][j]}, where @var{i} is the row number
>                               ^^^^^^^^^^^^^^^
> This should be @code{za[@var{i}][@var{j}]}.
> 

Fixed now.

>> +The @code{svg} register is the streaming vector granule (@code{SVG}) for the
>> +current thread. @xref{SVG}
> 
> Likewise: the @xref is not necessary.
> 

Removed.

>> +If the @sc{sm} bit is 1, it means the current thread is in streaming
>> +mode, and the SVE registers will have their sizes based on the @code{svg}
>> +register.  If the @sc{sm} bit is 0, the current thread is not in streaming
>> +mode, and the SVE registers have sizes based on the @code{vg} register.
>> +@xref{VG}. @xref{SVG}.
> 
> Here, only the cross-reference to VG is necessary.
> 

Fixed now.

>> +Setting the @code{svg} register to the same value will have no
>> +effect.
> 
> What do you mean by "same" here?  Do you mean to say that setting SVG
> is only meaningful if the value is different from its current value?
> 

It means if svg is 2 and you attempt to set it to 2 nothing will change in the
register state.  Then again, gdb already enforces this, I think.

I want to convey the information that the SME statw won't be reset if we accidentaly
set svg to the same value (possibly an implementation detail that shouldn't be in the
manual?).

>> +The possible values for @code{svg} are 2, 4, 8, 16, 32 (1, 2, 4, 8, 16
>> +for svq).
> 
> Are these values for the SVG register or for the SVG parameter?  Same
> question about the reference to "svq".
> 

The svg values are for both the register and the parameter. They are the same thing.

As stated before, svg (the register) is simply how gdb exposes SVG (the definition).

svq is only a concept. No registers expose it directly (though gdb uses this in the code).

svl is the exact same situation as svq. It is a concept and there is no register exposing its
value directly, but we use it internally in gdb's code.

This is an extension to what SVE already established with vg, vq and vl.

I can see where there is some confusion with the uppercase/lowercase uses of svq/svl. I've
rectified that. Given svq/svl don't have register counterparts, I think it is fair to always use
SVQ/SVL.

>>              These numbers correspond to streaming vector length values of 16
>> +bytes, 32 bytes, 64 bytes, 128 bytes and 256 bytes.
> 
> Same question here: are these the values of the SVL parameter?
> 

Yes. I describe the case for svg values (register/parameter), SVQ values and SVL values.

I've rephrased this slightly to...

The possible values for the @code{svg} register are 2, 4, 8, 16, 32 (1, 2, 4,
8, 16 for @code{SVQ}).  These numbers correspond to streaming vector length
(@code{SVL}) values of 16 bytes, 32 bytes, 64 bytes, 128 bytes and 256 bytes respectively.

>> +There is a fixed number of @code{za} tile pseudo registers (32).  They are
>> +@code{za0b}, @code{za0h} through @code{za1h}, @code{zas0} through @code{zas3},
>> +@code{zad0} through @code{zad7} and @code{zaq0} through @code{zaq15}.
> 
> Something is amiss here, I think, since I only get 31 when I add the
> above numbers (1+2+4+8+16).  What did I miss?
> 

Yeah, that's a typo. It should really be 31. Thanks for catching that.

I might have read this too many times.

>> +Tile slice pseudo-registers are vectors of horizontally or vertically
>> +contiguous elements within the @code{za} register.
>> +
>> +The tile slice pseudo-registers are numerous.  For a minimum streaming vector
>> +length of 16 bytes, there are 5 x 32 pseudo registers.  For the maximum
>> +streaming vector length of 256 bytes, there are 5 x 512 pseudo registers.
> 
> An explanation of why 5 and why 32 or 512 will help here, I think.
> 

I noticed I missed a part of the tile slice register naming. I've reordered this block
and rewritten some of it. How does it look?

The tile slice pseudo registers have the following naming pattern:
@code{za<@var{tile number}><@var{direction}>@var{qualifier}
<@var{slice number}>}.

There are up to 16 tiles (0 ~ 15), the direction can be either @code{v}
(vertical) or @code{h} (horizontal), the qualifiers can be @code{b} (byte),
@code{h} (halfword), @code{s} (word), @code{d} (doubleword) and @code{q}
(quadword) and there are up to 256 slices (0 ~ 255) depending on the value
of @code{svg}.  The number of slices is the same as the value of @code{SVL}.

The number of available tile slice pseudo-registers can be large.  For a
minimum @code{SVL} of 16 bytes, there are 5 (number of qualifiers) x
2 (number of directions) x 16 (@code{SVL}) pseudo registers.  For the
maximum @code{SVL} of 256 bytes, there are 5 x 2 x 256 pseudo registers.


>> +The tile slice pseudo registers have the following naming pattern:
>> +za<@var{tile number}><@var{orientation}><@var{slice number}>.
>> +
>> +There are up to 16 tiles (0 ~ 15), the orientation can be either vertical (v)
>> +or horizontal (h) and there are up to 256 slices (0 ~ 255) depending on the
>> +value of @code{svg}.
>     ^^^^^^^^^^^^^^^^^^^
> Is this the SVG parameter or the value of the SVG register?
> 

Same situation as before. Given SVG is represented by a register svg, I'm talking about the register.

>> +When listing all the available registers, users will see the
>> +currently-available @code{za} pseudo-registers.  Pseudo-registers that don't
>> +exist for a given @code{svg} value will not be displayed.
>                       ^^^^^^^^^^^^^^^^
> Same question here.
> 

Same as above.

>> +For more information on @acronym{SME} and its terminology, please refer to the
>> +@url{https://developer.arm.com/documentation/ddi0616/aa/,
>> +Arm Architecture Reference Manual Supplement}, The Scalable Matrix Extension
>> +(@acronym{SME}), for Armv9-A.
>> +
>> +Some features are still under development and rely on ACLE and ABI
>> +definitions, so there are known limitations to the current @acronym{SME}
>> +support in @value{GDBN}.
> 
> What is "ACLE"?  It is not used anywhere else in the manual, AFAICT.
> 

Arm C Language Extensions. I've added a link to it and to the ABI document, like so:

Some features are still under development and rely on
@url{https://github.com/ARM-software/acle/releases/latest, ACLE} and
@url{https://github.com/ARM-software/abi-aa/blob/main/aapcs64/aapcs64.rst, ABI}
definitions, so there are known limitations to the current @acronym{SME}
support in @value{GDBN}.

>> +One such example is calling functions by hand from @value{GDBN}.  Hand calls
>> +are not @acronym{SME}-aware and thus don't take into account the @code{svcr}
>> +bits nor the @code{za} contents.
> 
> I believe we refer to such calls as "calling functions in the program
> being debugged", not "calling by hand".  Also, a cross-reference to
> the "Calling" node would be beneficial here.
> 

Probably implementor speak here. I've fixed it now. It should say the following:

One such example is calling functions in the program being debugged by
@value{GDBN}.  Such calls are not @acronym{SME}-aware and thus don't take into
account the @code{svcr} pseudo-register bits nor the @code{za} register
contents.

>> +The lazy saving scheme involving the @code{tpidr2} register is not yet
>> +supported by @value{GDBN}, though the @code{tpidr2} register is known
>> +and supported by @value{GDBN}.
> 
> What is the "lazy saving scheme"?  If it's described somewhere in the
> manual, please use here the same terminology as there, and please
> include a cross-reference to that place.
> 

It is described in the ABI. I've added a link to the ABI document and the section
describing it:

The @url{https://github.com/ARM-software/abi-aa/blob/main/aapcs64/aapcs64.rst#the-za-lazy-saving-scheme,
lazy saving scheme} involving the @code{tpidr2} register is not yet supported
by @value{GDBN}, though the @code{tpidr2} register is known and supported
by @value{GDBN}.

>> +Lastly, an important limitation for @code{gdbserver} is its inability to
>> +communicate changes in the streaming vector length to @value{GDBN}.  This
>> +means @code{gdbserver}, even though it is capable of adjusting its internal
>> +caches to reflect a change to @code{svg}, will operate with a potentially
>> +different @code{svg} value compared to @value{GDBN}.  This can lead to
>> +@value{GDBN} showing incorrect values for the @code{za} register and
>> +incorrect values for SVE registers (when the @sc{m} bit is on).
> 
> Is it really reasonable to release this feature given this glaring
> limitation?
> 

I think it isn't great, but OK. This has been a limitation for SVE since its inception.  I proposed
changes to fix this issue a while ago, and, more recently, Thiago has picked up this work and is
working on a potential fix. Once fixed for the SVE case, it will be trivial to make it work for
SME as well. I think this is also a limitation for Intel AMX.

The rationale for it being OK is that changes to the SVE vector length and SME vector length that
happen mid-execution of a program are very unlikely. The most common scenario is a program using
a single vector length throughout its execution.

>> +@item
>> +@samp{za} is a vector of bytes of size svl x svl.
> 
> See the comments above: I'd use
> 
>    [...] rectangular matrix bytes of the size @code{@var{nsvl}x@var{nsvl}}
> 

I've adjusted this, provisionally, to the following:

@item
@samp{svg} is a 64-bit pseudo-register containing the number of 64-bit chunks
in @code{SVL}.  Its value is equal to @code{SVG}.  @xref{SVG}.  @xref{SVL}.

>> +@item
>> +@samp{svg} is a 64-bit pseudo register containing the number of 64-bit chunks
>> +in svl. @xref{SVG}
>           ^^
> Two spaces there.
> 

Fixed.

>> +@item
>> +@samp{svcr} is a 64-bit state register containing bits 0 (@sc{sm}) and
>> +1 (@sc{za}). @xref{aarch64 sme svcr}
>                ^^
> And there.
> 

Fixed.

> Reviewed-By: Eli Zaretskii <eliz@gnu.org>