| Message ID | 20230822093712.38922-1-philmd@linaro.org |
|---|---|
| Headers | show |
| Series | tcg: Document *swap/deposit helpers | expand |
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(-)
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
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).