Re: [PATCH 17/28] tcg: Add flags argument to tcg_gen_bswap16_*, tcg_gen_bswap32_i64

[Philippe Mathieu-Daudé]

On 6/14/21 10:37 AM, Richard Henderson wrote:
> Implement the new semantics in the fallback expansion.
> Change all callers to supply the flags that keep the
> semantics unchanged locally.
> 
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
>  include/tcg/tcg-op.h            |   8 +--
>  target/arm/translate-a64.c      |  12 ++--
>  target/arm/translate.c          |   2 +-
>  target/i386/tcg/translate.c     |   2 +-
>  target/mips/tcg/mxu_translate.c |   2 +-
>  target/s390x/translate.c        |   4 +-
>  target/sh4/translate.c          |   2 +-

Various REV 16/32, would it be useful to have it as a TCG opcode?

>  tcg/tcg-op.c                    | 121 ++++++++++++++++++++++----------
>  8 files changed, 99 insertions(+), 54 deletions(-)

>      } else {
>          TCGv_i64 t0 = tcg_temp_new_i64();
>          TCGv_i64 t1 = tcg_temp_new_i64();
>          TCGv_i64 t2 = tcg_constant_i64(0x00ff00ff);
>  
> -                                        /* arg = ....abcd */
> -        tcg_gen_shri_i64(t0, arg, 8);   /*  t0 = .....abc */
> -        tcg_gen_and_i64(t1, arg, t2);   /*  t1 = .....b.d */
> -        tcg_gen_and_i64(t0, t0, t2);    /*  t0 = .....a.c */
> -        tcg_gen_shli_i64(t1, t1, 8);    /*  t1 = ....b.d. */
> -        tcg_gen_or_i64(ret, t0, t1);    /* ret = ....badc */
> +                                            /* arg = xxxxabcd */
> +        tcg_gen_shri_i64(t0, arg, 8);       /*  t0 = .xxxxabc */
> +        tcg_gen_and_i64(t1, arg, t2);       /*  t1 = .....b.d */
> +        tcg_gen_and_i64(t0, t0, t2);        /*  t0 = .....a.c */
> +        tcg_gen_shli_i64(t1, t1, 8);        /*  t1 = ....b.d. */
> +        tcg_gen_or_i64(ret, t0, t1);        /* ret = ....badc */
>  
> -        tcg_gen_shli_i64(t1, ret, 48);  /*  t1 = dc...... */
> -        tcg_gen_shri_i64(t0, ret, 16);  /*  t0 = ......ba */
> -        tcg_gen_shri_i64(t1, t1, 32);   /*  t1 = ....dc.. */
> -        tcg_gen_or_i64(ret, t0, t1);    /* ret = ....dcba */
> +        tcg_gen_shli_i64(t1, ret, 48);      /*  t1 = dc...... */
> +        tcg_gen_shri_i64(t0, ret, 16);      /*  t0 = ......ba */
> +        if (flags & TCG_BSWAP_OS) {
> +            tcg_gen_sari_i64(t1, t1, 32);   /*  t1 = ssssdc.. */
> +        } else {
> +            tcg_gen_shri_i64(t1, t1, 32);   /*  t1 = ....dc.. */
> +        }
> +        tcg_gen_or_i64(ret, t0, t1);        /* ret = ssssdcba */

Comment update appreciated, thanks.

Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>