Re: [PATCH 1/7] tcg/tcg-op: Document bswap16() byte pattern

[Richard Henderson <richard.henderson@linaro.org>]

On 8/22/23 02:37, Philippe Mathieu-Daudé wrote:
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
>   tcg/tcg-op.c | 48 ++++++++++++++++++++++++++++++++----------------
>   1 file changed, 32 insertions(+), 16 deletions(-)
> 
> diff --git a/tcg/tcg-op.c b/tcg/tcg-op.c
> index 7aadb37756..f164ddc95e 100644
> --- a/tcg/tcg-op.c
> +++ b/tcg/tcg-op.c
> @@ -1021,6 +1021,13 @@ void tcg_gen_ext16u_i32(TCGv_i32 ret, TCGv_i32 arg)
>       }
>   }
>   
> +/*
> + * bswap16_i32: 16-bit byte swap on the low bits of a 32-bit value.
> + *
> + * Byte pattern:  bswap16_i32(xxab) -> ..ba             (TCG_BSWAP_OZ)
> + *                bswap16_i32(xxab) -> ssba             (TCG_BSWAP_OS)
> + *                bswap16_i32(xxab) -> xxba
> + */

Don't forget TCG_BSWAP_IZ, which means the input is already zero-extended.
Which makes

> +                                            /* arg = xxab */
> +        tcg_gen_shri_i32(t0, arg, 8);       /*  t0 = .xxa */

this

>           if (!(flags & TCG_BSWAP_IZ)) {
> -            tcg_gen_ext8u_i32(t0, t0);
> +            tcg_gen_ext8u_i32(t0, t0);      /*  t0 = ...a */
>           }
>   
>           if (flags & TCG_BSWAP_OS) {
> -            tcg_gen_shli_i32(t1, arg, 24);
> -            tcg_gen_sari_i32(t1, t1, 16);
> +            tcg_gen_shli_i32(t1, arg, 24);  /*  t1 = b... */
> +            tcg_gen_sari_i32(t1, t1, 16);   /*  t1 = ssb. */
>           } else if (flags & TCG_BSWAP_OZ) {
> -            tcg_gen_ext8u_i32(t1, arg);
> -            tcg_gen_shli_i32(t1, t1, 8);
> +            tcg_gen_ext8u_i32(t1, arg);     /*  t1 = ...b */
> +            tcg_gen_shli_i32(t1, t1, 8);    /*  t1 = ..b. */
>           } else {
> -            tcg_gen_shli_i32(t1, arg, 8);
> +            tcg_gen_shli_i32(t1, arg, 8);   /*  t1 = xab. */

and this slightly inaccurate.

>           }
>   
> -        tcg_gen_or_i32(ret, t0, t1);
> +        tcg_gen_or_i32(ret, t0, t1);        /* ret = ssba */

This one is just confusing, since each of the three cases above have different outputs.


r~




r~