Re: [PATCH 00/12] tcg: Factor hrev{32,64}_{i32,i64,tl} out

[Richard Henderson]

On 8/22/23 05:40, Philippe Mathieu-Daudé wrote:
> This series factor the "byteswap each halfword within a
> 32/64-bit value" code duplication as generic helpers.
>
> Modulo the documentation added, there is a good negative
> diff-stat, so I believe this is a win from a maintainance
> point of view.
>
> I used "hrev" to follow the other bswap/hswap/rev helpers
> but it isn't very descriptive, so any better name suggestion
> is welcomed.
> (In particular because there are other patterns I'd like to
> factor out and then naming is getting worse, such 'wrev').'

I applaud the code factor, but the names are poor.

The "h" does not match the size of the elements being swapped, which is "b".  The "32" 
is... what, the total count of bits modified?

Naming is hard, and I'm not sure what's best.

We have bswap32_i32, bswap32_i64, bswap64_i64.

Perhaps bswap16x2_i32, bswap16x4_i64, bswap16xN_tl, to indicate that we're bswaping 16-bit 
quantities, and "xN" to indicate that multiple 16-bit quantities are being swapped.

From your subjects, it would appear we don't need bswap16x2_i64, with the upper 32-bits 
zero/signed/undefined.  But if we did, we should provide a flags argument of TCG_BSWAP_*.

That then extends to hswap32x2_i64 to swap halfwords within multiple words for mips DSHW 
et al.


r~