Re: [PATCH] Add SVE to NEWS and GDB manual

[Alan Hayward <Alan.Hayward@arm.com>]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset="utf-8", Size: 6333 bytes --]

Thanks for the review.

> On 21 Aug 2018, at 15:36, Pedro Alves <palves@redhat.com> wrote:
> 
> On 08/21/2018 01:53 PM, Alan Hayward wrote:
>> I wasn't sure whether the vector length limitation should
>> get added to these files - I couldn't find any other limitations
>> stated in these files. Happy to remove those parts / move elsewhere.
>> 
> 
> 
>> Also corrected vector gradient to vector granule, as per DWARF spec.
> 
> That seems unrelated, best to apply as a separate patch, I'd think.

Ok, I’ll post and push that as it’s own OBV patch.

> 
>> 
>> Ok for 8.2 branch too?
>> 
>> 
>> 2018-08-21  Alan Hayward  <alan.hayward@arm.com>
>> 
>> gdb/
>> 	* NEWS: Add SVE to 8.2 section.
>> 	* doc/gdb.texinfo: Add SVE to AArch64 section.
>> 	* arch/aarch64.h (aarch64_regnum): Update comment.
>> ---
>> gdb/NEWS            |  3 +++
>> gdb/arch/aarch64.h  |  4 ++--
>> gdb/doc/gdb.texinfo | 16 ++++++++++++++++
>> 3 files changed, 21 insertions(+), 2 deletions(-)
>> 
>> diff --git a/gdb/NEWS b/gdb/NEWS
>> index 16d3d72589..3119d97d98 100644
>> --- a/gdb/NEWS
>> +++ b/gdb/NEWS
>> @@ -90,6 +90,9 @@ GNU/Linux/RISC-V		riscv*-*-linux*
>> * C expressions can now use _Alignof, and C++ expressions can now use
>>   alignof.
>> 
>> +* Support for SVE on AArch64 Linux.  Note that in 8.2, GDB does not detect
>> +  changes to the vector length while the process is running.
> 
> I assume this is the "new in 8.2" section, so is there a reason you're saying
> "note that in 8.2" instead of "note that" ??  It seems redundant.
> If it isn't redundant, and you mean to say that master (8.3) will
> behave differently, then such behavior should instead get its own
> news entry in 8.3.
> 

My worry was that for 8.3 and onwards, anyone quickly reading the NEWS file
might miss the entry 8.3 saying it now works. But, agree with your comment.
Removed the redundancy.


>> +
>> * New commands
>> 
>> set debug fbsd-nat
>> diff --git a/gdb/arch/aarch64.h b/gdb/arch/aarch64.h
>> index e344a36770..d6b88e6d56 100644
>> --- a/gdb/arch/aarch64.h
>> +++ b/gdb/arch/aarch64.h
>> @@ -49,7 +49,7 @@ enum aarch64_regnum
>>   AARCH64_SVE_P15_REGNUM = AARCH64_SVE_P0_REGNUM + 15,	/* Last SVE predicate
>> 							   register.  */
>>   AARCH64_SVE_FFR_REGNUM,	/* SVE First Fault Register.  */
>> -  AARCH64_SVE_VG_REGNUM,	/* SVE Vector Gradient.  */
>> +  AARCH64_SVE_VG_REGNUM,	/* SVE Vector Granule.  */
>> 
>>   /* Other useful registers.  */
>>   AARCH64_LAST_X_ARG_REGNUM = AARCH64_X0_REGNUM + 7,
>> @@ -71,7 +71,7 @@ enum aarch64_regnum
>> 	The number of bytes in an SVE Z register.
>>    VQ : Vector Quotient.
>> 	The number of 128bit chunks in an SVE Z register.
>> -   VG : Vector Gradient.
>> +   VG : Vector Granule.
>> 	The number of 64bit chunks in an SVE Z register.  */
>> 
>> #define sve_vg_from_vl(vl)	((vl) / 8)
>> diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
>> index 433a2698a9..4526a385d0 100644
>> --- a/gdb/doc/gdb.texinfo
>> +++ b/gdb/doc/gdb.texinfo
>> @@ -23451,6 +23451,22 @@ Show whether AArch64 debugging messages are displayed.
>> 
>> @end table
>> 
>> +@subsubsection AArch64 SVE.
>> +@cindex AArch64 SVE.
>> +
>> +When GDB is debugging the AArch64 architecture, if the Scalable Vector
> 
> Throughout, you should use @value{GDBN} instead of "GDB”.

Done.

> 
>> +Extension (SVE) is present then GDB will provide the vector registers
>> +@code{$z0} through @code{$z31}, vector predicate registers @code{$p0} through
>> +@code{$p15}, and the @code{$ffr} register.  In addition the pseduo register
> 
> Typo: "pseduo" -> "pseudo”.

Oops. Done.

> 
>> +@code{$vg} will be provided.  This is the vector granule for the current thread
>> +and represents the number of 64-bit chunks in a SVE Z register.
> 
> Should that be "an SVE" (as in, "an ess, vee, eee") ?  Or is SVE usually read
> as "a ssssveeee”?

Had a quick chat with our documentation team. Updated to “an”.

> 
>> +
>> +If the vector length changes, then the @code{$vg} register will be updated,
>> +but the lengths of the @code{z} and @code{p} registers will not change. This
>> +is a known limitation of GDB and does not affect the execution of the target
>> +process.
> 
> Is the documentation for the "org.gnu.gdb.aarch64.sve" XML tdesc feature in the
> branch already?
> 

Yes, added as part of the SVE target description patch.


Updated version below.
This version ok?



2018-08-21  Alan Hayward  <alan.hayward@arm.com>

gdb/
	* NEWS: Add SVE to 8.2 section.
	* doc/gdb.texinfo: Add SVE to AArch64 section.



diff --git a/gdb/NEWS b/gdb/NEWS
index 16d3d725897c8c06124709c0203be5d24c85f72b..bd03d72e9afb2b5413b5a322754fc352ee7c34dc 100644
--- a/gdb/NEWS
+++ b/gdb/NEWS
@@ -90,6 +90,9 @@ GNU/Linux/RISC-V		riscv*-*-linux*
 * C expressions can now use _Alignof, and C++ expressions can now use
   alignof.

+* Support for SVE on AArch64 Linux.  Note that GDB does not detect changes to
+  the vector length while the process is running.
+
 * New commands

 set debug fbsd-nat
diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
index 433a2698a9264b352e560259f6157dc98a6777e3..e58d67283b341de9866ed1605f3285bf7c81ce78 100644
--- a/gdb/doc/gdb.texinfo
+++ b/gdb/doc/gdb.texinfo
@@ -23451,6 +23451,22 @@ Show whether AArch64 debugging messages are displayed.

 @end table

+@subsubsection AArch64 SVE.
+@cindex AArch64 SVE.
+
+When @value{GDBN} is debugging the AArch64 architecture, if the Scalable Vector
+Extension (SVE) is present then @value{GDBN} will provide the vector registers
+@code{$z0} through @code{$z31}, vector predicate registers @code{$p0} through
+@code{$p15}, and the @code{$ffr} register.  In addition the pseudo register
+@code{$vg} will be provided.  This is the vector granule for the current thread
+and represents the number of 64-bit chunks in an SVE @code{z} register.
+
+If the vector length changes, then the @code{$vg} register will be updated,
+but the lengths of the @code{z} and @code{p} registers will not change.  This
+is a known limitation of @value{GDBN} and does not affect the execution of the
+target process.
+
+
 @node i386
 @subsection x86 Architecture-specific Issues


\x16º&Öéj×!zÊÞ¶êç׏}çYb²Ö«r\x18\x1dn–­r\x17¬