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

[Eli Zaretskii <eliz@gnu.org>]

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

> +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 [...]

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

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

> +@xref{SVL}

This reference is unnecessary, as the description is only a short ways
above, and in the same node.

> +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}]}.

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

Likewise: the @xref is not necessary.

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

> +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?

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

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

> +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?

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

> +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?

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

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

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

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

> +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?

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

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

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

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