Re: [Qemu-devel] [PATCH 04/11] target-mips: convert bitfield ops to TCG

["Laurent Desnogues" <laurent.desnogues@gmail.com>]

On Sat, Nov 8, 2008 at 9:34 AM, Aurelien Jarno <aurelien@aurel32.net> wrote:
> Bitfield operations can be written with very few TCG instructions
> (between 2 and 5), so it is worth converting them to TCG.
>
> Signed-off-by: Aurelien Jarno <aurelien@aurel32.net>
> ---
>  target-mips/helper.h    |    6 +----
>  target-mips/op_helper.c |   26 +------------------------
>  target-mips/translate.c |   49 +++++++++++++++++++++++++++++++++++++---------
>  3 files changed, 41 insertions(+), 40 deletions(-)
[...]
> diff --git a/target-mips/translate.c b/target-mips/translate.c
> index af01f73..2cd1868 100644
> --- a/target-mips/translate.c
> +++ b/target-mips/translate.c
> @@ -2682,57 +2682,86 @@ static void gen_compute_branch (DisasContext *ctx, uint32_t opc,
>  static void gen_bitops (DisasContext *ctx, uint32_t opc, int rt,
>                         int rs, int lsb, int msb)
>  {
> -    TCGv t0 = tcg_temp_local_new(TCG_TYPE_TL);
> -    TCGv t1 = tcg_temp_local_new(TCG_TYPE_TL);
> +    TCGv t0 = tcg_temp_new(TCG_TYPE_TL);
> +    TCGv t1 = tcg_temp_new(TCG_TYPE_TL);
> +    target_ulong mask;
>
>     gen_load_gpr(t1, rs);
>     switch (opc) {
>     case OPC_EXT:
>         if (lsb + msb > 31)
>             goto fail;
> -        tcg_gen_helper_1_1ii(do_ext, t0, t1, lsb, msb + 1);
> +        tcg_gen_shri_tl(t0, t1, lsb);
> +        if (msb + 1 < 32) {

Given the above restriction of lsb + msb <= 31, this test can
be rewritten as:

     if (msb != 31)

I find this more readable, but that's personal taste :-)

> +            tcg_gen_andi_tl(t0, t0, (1 << (msb + 1)) - 1);
> +        } else {
> +            tcg_gen_ext32s_tl(t0, t0);
> +        }
>         break;
>  #if defined(TARGET_MIPS64)
>     case OPC_DEXTM:
>         if (lsb + msb > 63)
>             goto fail;

Can this really happen?  lsb and msb are 5 bit wide as far
as I could see.

> -        tcg_gen_helper_1_1ii(do_dext, t0, t1, lsb, msb + 1 + 32);
> +        tcg_gen_shri_tl(t0, t1, lsb);
> +        if (msb + 1 + 32 < 64) {

This can be rewritten as

    if (msb != 31)

> +            tcg_gen_andi_tl(t0, t0, (1ULL << (msb + 1 + 32)) - 1);
> +        }
>         break;
>     case OPC_DEXTU:
>         if (lsb + msb > 63)
>             goto fail;

Same as above.

> -        tcg_gen_helper_1_1ii(do_dext, t0, t1, lsb + 32, msb + 1);
> +        tcg_gen_shri_tl(t0, t1, lsb + 32);
> +        tcg_gen_andi_tl(t0, t0, (1ULL << (msb + 1)) - 1);
>         break;
>     case OPC_DEXT:
>         if (lsb + msb > 63)
>             goto fail;

Same as above.


Laurent