Re: [Qemu-devel] [PATCH 46/60] AArch64: Add rev instruction family emulation

[Richard Henderson <rth@twiddle.net>]

On 09/26/2013 05:48 PM, Alexander Graf wrote:
> This patch adds emlulation support for rev and rbit instructions.
> 
> Signed-off-by: Alexander Graf <agraf@suse.de>
> ---
>  target-arm/helper-a64.c    | 19 +++++++++++++++++++
>  target-arm/helper-a64.h    |  1 +
>  target-arm/translate-a64.c | 38 ++++++++++++++++++++++++++++++++++++++
>  3 files changed, 58 insertions(+)
> 
> diff --git a/target-arm/helper-a64.c b/target-arm/helper-a64.c
> index a56ce75..e20b89f 100644
> --- a/target-arm/helper-a64.c
> +++ b/target-arm/helper-a64.c
> @@ -237,3 +237,22 @@ int64_t HELPER(sdiv64)(int64_t num, int64_t den)
>        return LLONG_MIN;
>      return num / den;
>  }
> +
> +uint64_t HELPER(rbit64)(uint64_t x)
> +{
> +    x =  ((x & 0xff00000000000000ULL) >> 56)
> +       | ((x & 0x00ff000000000000ULL) >> 40)
> +       | ((x & 0x0000ff0000000000ULL) >> 24)
> +       | ((x & 0x000000ff00000000ULL) >> 8)
> +       | ((x & 0x00000000ff000000ULL) << 8)
> +       | ((x & 0x0000000000ff0000ULL) << 24)
> +       | ((x & 0x000000000000ff00ULL) << 40)
> +       | ((x & 0x00000000000000ffULL) << 56);

This first step is of course bswap64, no?

> +    case 0x0: /* RBIT */
> +        if (is_32bit) {
> +            tcg_tmp = tcg_temp_new_i32();
> +            tcg_gen_trunc_i64_i32(tcg_tmp, cpu_reg(rn));
> +            gen_helper_rbit(tcg_tmp, tcg_tmp);
> +            tcg_gen_extu_i32_i64(cpu_reg(rd), tcg_tmp);
> +            tcg_temp_free_i32(tcg_tmp);
> +        } else {
> +            gen_helper_rbit64(cpu_reg(rd), cpu_reg(rn));
> +        }

I suppose that works.  Alternately, compute as

    rd = rbit64(rn << 32);

> +    case 0x1: /* REV16 */
> +        tcg_gen_bswap16_i64(cpu_reg(rd), cpu_reg(rn));
> +        break;
> +    case 0x2: /* REV32 */
> +        tcg_gen_bswap32_i64(cpu_reg(rd), cpu_reg(rn));
> +        break;

Aren't these two wrong?  Doesn't revN swap pairs of N bits
all the way up the register?  See gen_rev16 for A32.

Certainly one could use bswap32_i64 for REV32 if is_32bit.


r~