Re: [Qemu-devel] [PATCH 02/13] target-alpha: Implement cpys{, n, e} inline.

[Aurelien Jarno <aurelien@aurel32.net>]

On Fri, Mar 12, 2010 at 11:22:45AM -0800, Richard Henderson wrote:
> Signed-off-by: Richard Henderson <rth@twiddle.net>
> ---
>  target-alpha/helper.h    |    4 --
>  target-alpha/op_helper.c |   18 ----------
>  target-alpha/translate.c |   78 +++++++++++++++++++++++++++++++++++++++++++--
>  3 files changed, 74 insertions(+), 26 deletions(-)
> 
> diff --git a/target-alpha/helper.h b/target-alpha/helper.h
> index a508077..8e11304 100644
> --- a/target-alpha/helper.h
> +++ b/target-alpha/helper.h
> @@ -77,10 +77,6 @@ DEF_HELPER_FLAGS_2(cmpgeq, TCG_CALL_CONST | TCG_CALL_PURE, i64, i64, i64)
>  DEF_HELPER_FLAGS_2(cmpgle, TCG_CALL_CONST | TCG_CALL_PURE, i64, i64, i64)
>  DEF_HELPER_FLAGS_2(cmpglt, TCG_CALL_CONST | TCG_CALL_PURE, i64, i64, i64)
>  
> -DEF_HELPER_FLAGS_2(cpys, TCG_CALL_CONST | TCG_CALL_PURE, i64, i64, i64)
> -DEF_HELPER_FLAGS_2(cpysn, TCG_CALL_CONST | TCG_CALL_PURE, i64, i64, i64)
> -DEF_HELPER_FLAGS_2(cpyse, TCG_CALL_CONST | TCG_CALL_PURE, i64, i64, i64)
> -
>  DEF_HELPER_FLAGS_1(cvtts, TCG_CALL_CONST, i64, i64)
>  DEF_HELPER_FLAGS_1(cvtst, TCG_CALL_CONST, i64, i64)
>  DEF_HELPER_FLAGS_1(cvtqs, TCG_CALL_CONST, i64, i64)
> diff --git a/target-alpha/op_helper.c b/target-alpha/op_helper.c
> index 4d2c2ee..2419dc4 100644
> --- a/target-alpha/op_helper.c
> +++ b/target-alpha/op_helper.c
> @@ -921,24 +921,6 @@ uint64_t helper_sqrtt (uint64_t a)
>      return float64_to_t(fr);
>  }
>  
> -
> -/* Sign copy */
> -uint64_t helper_cpys(uint64_t a, uint64_t b)
> -{
> -    return (a & 0x8000000000000000ULL) | (b & ~0x8000000000000000ULL);
> -}
> -
> -uint64_t helper_cpysn(uint64_t a, uint64_t b)
> -{
> -    return ((~a) & 0x8000000000000000ULL) | (b & ~0x8000000000000000ULL);
> -}
> -
> -uint64_t helper_cpyse(uint64_t a, uint64_t b)
> -{
> -    return (a & 0xFFF0000000000000ULL) | (b & ~0xFFF0000000000000ULL);
> -}
> -
> -
>  /* Comparisons */
>  uint64_t helper_cmptun (uint64_t a, uint64_t b)
>  {
> diff --git a/target-alpha/translate.c b/target-alpha/translate.c
> index 719b423..b677378 100644
> --- a/target-alpha/translate.c
> +++ b/target-alpha/translate.c
> @@ -741,6 +741,80 @@ static inline void glue(gen_f, name)(DisasContext *ctx,         \
>  IEEE_INTCVT(cvtqs)
>  IEEE_INTCVT(cvtqt)
>  
> +static void gen_cpys_internal(int ra, int rb, int rc, int inv_a, uint64_t mask)
> +{
> +    TCGv va, vb, vmask;
> +    int za = 0, zb = 0;
> +
> +    if (unlikely(rc == 31)) {
> +        return;
> +    }
> +
> +    vmask = tcg_const_i64(mask);
> +
> +    TCGV_UNUSED_I64(va);
> +    if (ra == 31) {
> +        if (inv_a) {
> +            va = vmask;
> +        } else {
> +            za = 1;
> +        }
> +    } else {
> +        va = tcg_temp_new_i64();
> +        tcg_gen_mov_i64(va, cpu_fir[ra]);
> +        if (inv_a) {
> +            tcg_gen_not_i64(va, va);
> +        }
> +        tcg_gen_and_i64(va, va, vmask);

You can use instead:
    if (inv_a) {
       tcg_gen_andc_i64(va, vmask, va);
    } else {
       tcg_gen_and_i64(va, vmask, va);
    }

> +    }
> +
> +    TCGV_UNUSED_I64(vb);
> +    if (rb == 31) {
> +        zb = 1;
> +    } else {
> +        vb = tcg_temp_new_i64();
> +        tcg_gen_andc_i64(vb, cpu_fir[rb], vmask);
> +    }
> +
> +    switch (za * 2 + zb) {
> +    case 0:
> +        tcg_gen_or_i64(cpu_fir[rc], va, vb);
> +        break;
> +    case 1:
> +        tcg_gen_mov_i64(cpu_fir[rc], va);
> +        break;
> +    case 2:
> +        tcg_gen_mov_i64(cpu_fir[rc], vb);
> +        break;
> +    case 3:
> +        tcg_gen_movi_i64(cpu_fir[rc], 0);
> +        break;
> +    }

It's probably more clear here if you use switch(za << 1 | zb) and later
case 0 | 0:, case 0 | 1:, case 2 | 0 and case 2 | 1:.

> +    tcg_temp_free(vmask);
> +    if (ra != 31) {
> +        tcg_temp_free(va);
> +    }
> +    if (rb != 31) {
> +        tcg_temp_free(vb);
> +    }
> +}
> +
> +static inline void gen_fcpys(int ra, int rb, int rc)
> +{
> +    gen_cpys_internal(ra, rb, rc, 0, 0x8000000000000000ULL);
> +}
> +
> +static inline void gen_fcpysn(int ra, int rb, int rc)
> +{
> +    gen_cpys_internal(ra, rb, rc, 1, 0x8000000000000000ULL);
> +}
> +
> +static inline void gen_fcpyse(int ra, int rb, int rc)
> +{
> +    gen_cpys_internal(ra, rb, rc, 0, 0xFFF0000000000000ULL);
> +}
> +
>  #define FARITH3(name)                                           \
>  static inline void glue(gen_f, name)(int ra, int rb, int rc)    \
>  {                                                               \
> @@ -769,10 +843,6 @@ static inline void glue(gen_f, name)(int ra, int rb, int rc)    \
>          tcg_temp_free(vb);                                      \
>      }                                                           \
>  }
> -/* ??? Ought to expand these inline; simple masking operations.  */
> -FARITH3(cpys)
> -FARITH3(cpysn)
> -FARITH3(cpyse)
>  
>  /* ??? VAX instruction qualifiers ignored.  */
>  FARITH3(addf)
> -- 
> 1.6.6.1
> 
> 
> 
> 

-- 
Aurelien Jarno                          GPG: 1024D/F1BCDB73
aurelien@aurel32.net                 http://www.aurel32.net