While reviewing a recent patch from Richard optimizing
deposit() [*] I ended looking at the *swap friends, taking
some notes, which then evolved to proper documentation.

[*] https://lore.kernel.org/qemu-devel/20230816145547.477974-3-richard.henderson@linaro.org/

Philippe Mathieu-Daudé (7):
  tcg/tcg-op: Document bswap16() byte pattern
  tcg/tcg-op: Document bswap32() byte pattern
  tcg/tcg-op: Document bswap64() byte pattern
  tcg/tcg-op: Document hswap() byte pattern
  tcg/tcg-op: Document wswap() byte pattern
  tcg/tcg-op: Document deposit_z()
  target/cris: Fix a typo in gen_swapr()

 docs/devel/tcg-ops.rst  | 12 ++++++
 target/cris/translate.c | 20 +++++----
 tcg/tcg-op.c            | 96 +++++++++++++++++++++++++++++++----------
 3 files changed, 96 insertions(+), 32 deletions(-)

-- 
2.41.0

Philippe Mathieu-Daudé <philmd@linaro.org> writes:

> While reviewing a recent patch from Richard optimizing
> deposit() [*] I ended looking at the *swap friends, taking
> some notes, which then evolved to proper documentation.
>
> [*]
> https://lore.kernel.org/qemu-devel/20230816145547.477974-3-richard.henderson@linaro.org/

We already have some documentation in tcg.rst:

   * - bswap16_i32/i64 *t0*, *t1*, *flags*

     - | 16 bit byte swap on the low bits of a 32/64 bit input.
       |
       | If *flags* & ``TCG_BSWAP_IZ``, then *t1* is known to be zero-extended from bit 15.
       | If *flags* & ``TCG_BSWAP_OZ``, then *t0* will be zero-extended from bit 15.
       | If *flags* & ``TCG_BSWAP_OS``, then *t0* will be sign-extended from bit 15.
       |
       | If neither ``TCG_BSWAP_OZ`` nor ``TCG_BSWAP_OS`` are set, then the bits of *t0* above bit 15 may contain any value.

   * - bswap32_i64 *t0*, *t1*, *flags*

     - | 32 bit byte swap on a 64-bit value.  The flags are the same as for bswap16,
         except they apply from bit 31 instead of bit 15.

   * - bswap32_i32 *t0*, *t1*, *flags*

       bswap64_i64 *t0*, *t1*, *flags*

     - | 32/64 bit byte swap. The flags are ignored, but still present
         for consistency with the other bswap opcodes.

In an ideal world we could generate kdoc from the source file and
include it in the rest of the tcg docs. I'm not sure if it worth the
churn though? Richard?

https://qemu.readthedocs.io/en/master/devel/tcg-ops.html

>
> Philippe Mathieu-Daudé (7):
>   tcg/tcg-op: Document bswap16() byte pattern
>   tcg/tcg-op: Document bswap32() byte pattern
>   tcg/tcg-op: Document bswap64() byte pattern
>   tcg/tcg-op: Document hswap() byte pattern
>   tcg/tcg-op: Document wswap() byte pattern
>   tcg/tcg-op: Document deposit_z()
>   target/cris: Fix a typo in gen_swapr()
>
>  docs/devel/tcg-ops.rst  | 12 ++++++
>  target/cris/translate.c | 20 +++++----
>  tcg/tcg-op.c            | 96 +++++++++++++++++++++++++++++++----------
>  3 files changed, 96 insertions(+), 32 deletions(-)


-- 
Alex Bennée
Virtualisation Tech Lead @ Linaro

On 22/8/23 15:42, Alex Bennée wrote:
> 
> Philippe Mathieu-Daudé <philmd@linaro.org> writes:
> 
>> While reviewing a recent patch from Richard optimizing
>> deposit() [*] I ended looking at the *swap friends, taking
>> some notes, which then evolved to proper documentation.
>>
>> [*]
>> https://lore.kernel.org/qemu-devel/20230816145547.477974-3-richard.henderson@linaro.org/
> 
> We already have some documentation in tcg.rst:
> 
>     * - bswap16_i32/i64 *t0*, *t1*, *flags*
> 
>       - | 16 bit byte swap on the low bits of a 32/64 bit input.
>         |
>         | If *flags* & ``TCG_BSWAP_IZ``, then *t1* is known to be zero-extended from bit 15.
>         | If *flags* & ``TCG_BSWAP_OZ``, then *t0* will be zero-extended from bit 15.
>         | If *flags* & ``TCG_BSWAP_OS``, then *t0* will be sign-extended from bit 15.
>         |
>         | If neither ``TCG_BSWAP_OZ`` nor ``TCG_BSWAP_OS`` are set, then the bits of *t0* above bit 15 may contain any value.
> 
>     * - bswap32_i64 *t0*, *t1*, *flags*
> 
>       - | 32 bit byte swap on a 64-bit value.  The flags are the same as for bswap16,
>           except they apply from bit 31 instead of bit 15.
> 
>     * - bswap32_i32 *t0*, *t1*, *flags*
> 
>         bswap64_i64 *t0*, *t1*, *flags*
> 
>       - | 32/64 bit byte swap. The flags are ignored, but still present
>           for consistency with the other bswap opcodes.

I guess I wasn't clear enough: I mostly documented the implementation,
not the API.

That said, maybe the bytes movement pattern belong to the API doc
(at least I find it very useful to find which TCG opcode implement
the frontend code, when the TCG opcode name isn't trivial, although
documented).

On Tue, 22 Aug 2023 at 14:46, Alex Bennée <alex.bennee@linaro.org> wrote:
>
>
> Philippe Mathieu-Daudé <philmd@linaro.org> writes:
>
> > While reviewing a recent patch from Richard optimizing
> > deposit() [*] I ended looking at the *swap friends, taking
> > some notes, which then evolved to proper documentation.
> >
> > [*]
> > https://lore.kernel.org/qemu-devel/20230816145547.477974-3-richard.henderson@linaro.org/
>
> We already have some documentation in tcg.rst:
>
>    * - bswap16_i32/i64 *t0*, *t1*, *flags*
>
>      - | 16 bit byte swap on the low bits of a 32/64 bit input.
>        |
>        | If *flags* & ``TCG_BSWAP_IZ``, then *t1* is known to be zero-extended from bit 15.
>        | If *flags* & ``TCG_BSWAP_OZ``, then *t0* will be zero-extended from bit 15.
>        | If *flags* & ``TCG_BSWAP_OS``, then *t0* will be sign-extended from bit 15.
>        |
>        | If neither ``TCG_BSWAP_OZ`` nor ``TCG_BSWAP_OS`` are set, then the bits of *t0* above bit 15 may contain any value.
>
>    * - bswap32_i64 *t0*, *t1*, *flags*
>
>      - | 32 bit byte swap on a 64-bit value.  The flags are the same as for bswap16,
>          except they apply from bit 31 instead of bit 15.
>
>    * - bswap32_i32 *t0*, *t1*, *flags*
>
>        bswap64_i64 *t0*, *t1*, *flags*
>
>      - | 32/64 bit byte swap. The flags are ignored, but still present
>          for consistency with the other bswap opcodes.
>
> In an ideal world we could generate kdoc from the source file and
> include it in the rest of the tcg docs. I'm not sure if it worth the
> churn though? Richard?

I do think it would be useful to have documentation of the
set of APIs you use as a writer of a TCG frontend. This is
often not exactly the same as the TCG IR opcodes. (Similarly
what you have to do as a backend isn't exactly the same,
but the documentation need is less pressing because fewer
people need to work on the backends.)

thanks
-- PMM