Re: [PATCH,obv] [AArch64] Remove unused struct

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

Hi,

On 8/6/22 08:39, Enze Li wrote:
> On Fri, 2022-08-05 at 16:48 +0100, Luis Machado via Gdb-patches wrote:
>> While doing something else, I noticed struct sve_context is not used
>> anywhere.
>>
>> Remove it then.
>> ---
>>   gdb/nat/aarch64-sve-linux-sigcontext.h | 6 ------
>>   1 file changed, 6 deletions(-)
>>
>> diff --git a/gdb/nat/aarch64-sve-linux-sigcontext.h
>> b/gdb/nat/aarch64-sve-linux-sigcontext.h
>> index 1b40ffa8ee9..a166fbda7da 100644
>> --- a/gdb/nat/aarch64-sve-linux-sigcontext.h
>> +++ b/gdb/nat/aarch64-sve-linux-sigcontext.h
>> @@ -21,12 +21,6 @@
>>   
>>   #define SVE_MAGIC      0x53564501
>>   
>> -struct sve_context {
>> -       struct _aarch64_ctx head;
>> -       __u16 vl;
>> -       __u16 __reserved[3];
>> -};
>> -
>>   /*
>>    * The SVE architecture leaves space for future expansion of the
>>    * vector length beyond its initial architectural limit of 2048 bits
> 
> Hi Luis,
> 
> With this patch applied, I use the following command to check,
> 
> $ grep -rn --exclude='ChangeLog*' 'sve_context' .
> 
> ./gdb/nat/aarch64-sve-linux-sigcontext.h:48: * sve_context.head.size >=
> ./gdb/nat/aarch64-sve-linux-sigcontext.h:49:
> *	SVE_SIG_CONTEXT_SIZE(sve_vq_from_vl(sve_context.vl))
> ./gdb/nat/aarch64-sve-linux-sigcontext.h:52: * If sve_context.head.size
> <
> ./gdb/nat/aarch64-sve-linux-sigcontext.h:53:
> *	SVE_SIG_CONTEXT_SIZE(sve_vq_from_vl(sve_context.vl)),
> ./gdb/nat/aarch64-sve-linux-sigcontext.h:59: * will need to remove or
> resize the sve_context block if it wants to
> ./gdb/nat/aarch64-sve-linux-sigcontext.h:65: * sve_context.vl must
> equal the thread's current vector length when
> ./gdb/nat/aarch64-sve-linux-sigcontext.h:74: * guaranteed for a struct
> sve_context written by the kernel.
> ./gdb/nat/aarch64-sve-linux-sigcontext.h:79: * the start of struct
> sve_context, and SVE_SIG_x_SIZE(args) is the
> ./gdb/nat/aarch64-sve-linux-sigcontext.h:102:	
> ((sizeof(struct sve_context) + (SVE_VQ_BYTES - 1))
>                  ^^^^^^^^^^^
> I noticed that struct sve_context is used here, which is eventually
> used by SVE_SIG_CONTEXT_SIZE.  Although SVE_SIG_CONTEXT_SIZE is not
> currently in use, it could cause a lot of unnecessary trouble once
> someone uses it.

Indeed. That should be cleaned up as well. I failed to catch this on a build check. Thanks for
spotting it!

> 
> Other than that, there are still some comments that refer to struct
> sve_context, please see above.  Maybe it's overkill, it would be nice
> if you could deal with it.

No, that makes sense. The documentation can refer to the struct, but it should probably point out
at the source of the information, which is the Linux Kernel documentation.

> 
> Thanks,
> Enze
>