Re: [RFC PATCH 08/25] qemu/bswap: Use ST_CONVERT() macro to emit 16-bit load/store functions

["Philippe Mathieu-Daudé" <philmd@redhat.com>]

On 5/18/21 10:08 PM, Peter Maydell wrote:
> On Tue, 18 May 2021 at 19:37, Philippe Mathieu-Daudé <philmd@redhat.com> wrote:
>>
>> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
>> ---
>>  include/qemu/bswap.h | 17 ++---------------
>>  1 file changed, 2 insertions(+), 15 deletions(-)
>>
>> diff --git a/include/qemu/bswap.h b/include/qemu/bswap.h
>> index 86f5ded6acf..4e2bd2e97ee 100644
>> --- a/include/qemu/bswap.h
>> +++ b/include/qemu/bswap.h
>> @@ -350,11 +350,6 @@ static inline int ldsw_he_p(const void *ptr)
>>      return r;
>>  }
>>
>> -static inline void stw_he_p(void *ptr, uint16_t v)
>> -{
>> -    __builtin_memcpy(ptr, &v, sizeof(v));
>> -}
>> -
>>  static inline int ldl_he_p(const void *ptr)
>>  {
>>      int32_t r;
>> @@ -399,11 +394,6 @@ static inline uint64_t ldq_le_p(const void *ptr)
>>      return le_bswap(ldq_he_p(ptr), 64);
>>  }
>>
>> -static inline void stw_le_p(void *ptr, uint16_t v)
>> -{
>> -    stw_he_p(ptr, le_bswap(v, 16));
>> -}
>> -
>>  static inline void stl_le_p(void *ptr, uint32_t v)
>>  {
>>      stl_he_p(ptr, le_bswap(v, 32));
>> @@ -434,11 +424,6 @@ static inline uint64_t ldq_be_p(const void *ptr)
>>      return be_bswap(ldq_he_p(ptr), 64);
>>  }
>>
>> -static inline void stw_be_p(void *ptr, uint16_t v)
>> -{
>> -    stw_he_p(ptr, be_bswap(v, 16));
>> -}
>> -
>>  static inline void stl_be_p(void *ptr, uint32_t v)
>>  {
>>      stl_he_p(ptr, be_bswap(v, 32));
>> @@ -466,6 +451,8 @@ static inline void st ## size ## _ ## endian ## _p(void *ptr, vtype v)\
>>      ST_CONVERT_END(le, bits, vtype, size)\
>>      ST_CONVERT_END(be, bits, vtype, size)
>>
>> +ST_CONVERT(16, uint16_t, w)
>> +
> 
> Where we have a macro that emits a bunch of function declarations,
> can we also add a comment that (a) documents the functions and
> (b) explicitly lists the name of every generated function?
> The latter in particular is really helpful for people who are
> trying to find function declarations/definitions with 'grep'.
> The comment above the definition and use of the CPU_CONVERT
> macro in bswap.h is an example.

For (a) I think the current comment is still valid:

/*
 * the generic syntax is:
 *
 * load: ld{type}{sign}{size}_{endian}_p(ptr)
 *
 * store: st{type}{size}_{endian}_p(ptr, val)
 ...

For (b), certainly, I'll add that list.

Thanks,

Phil.