Re: [Qemu-devel] [PATCH 5/7] tcg-sparc: Implement setcond, movcond, setcond2, brcond2.

[Blue Swirl]

On Wed, Dec 16, 2009 at 11:26 PM, Richard Henderson <address@hidden> wrote:
> An initial cut at conditional moves for the sparc backend.
>
> Untested, as I don't have sparc hardware and the build system
> resists attempts at cross-compilation.

I can try if you have a test case.

> Note fixes to tcg_out_movi_imm32 (wrong check_fit_tl width),
> use of TCG_TARGET_REG_BITS == 64 tests instead of explicitly
> checking for __sparc_v9__ everywhere.

Good fixes. I think these should be in a different patch which could be applied.

> -    tcg_out_arith(s, ret, arg, TCG_REG_G0, ARITH_OR);
> +    if (ret != arg)
> +        tcg_out_arith(s, ret, arg, TCG_REG_G0, ARITH_OR);
>  }

This optimization is already handled at tcg-op.h:tcg_gen_mov_i32().

>  static inline void tcg_out_movi_imm32(TCGContext *s, int ret, uint32_t arg)
>  {
> -    if (check_fit_tl(arg, 12))
> +    if (check_fit_tl(arg, 13))
>         tcg_out_movi_imm13(s, ret, arg);

IIRC sign extension prevents this.

>  static inline void tcg_out_movi(TCGContext *s, TCGType type,
>                                 int ret, tcg_target_long arg)
>  {
> -#if defined(__sparc_v9__) && !defined(__sparc_v8plus__)
> -    if (!check_fit_tl(arg, 32) && (arg & ~0xffffffffULL) != 0) {
> -        tcg_out_movi_imm32(s, TCG_REG_I4, arg >> 32);
> -        tcg_out_arithi(s, TCG_REG_I4, TCG_REG_I4, 32, SHIFT_SLLX);
> +    if (type == TCG_TYPE_I32 || (arg & ~(tcg_target_long)0xffffffff))
>         tcg_out_movi_imm32(s, ret, arg);
> -        tcg_out_arith(s, ret, ret, TCG_REG_I4, ARITH_OR);
> -    } else if (check_fit_tl(arg, 12))
> -        tcg_out_movi_imm13(s, ret, arg);
> -    else {
> -        tcg_out_sethi(s, ret, arg);
> -        if (arg & 0x3ff)
> -            tcg_out_arithi(s, ret, ret, arg & 0x3ff, ARITH_OR);
> +    else if (TCG_TARGET_REG_BITS == 64) {
> +        if (check_fit_tl(arg, 32)) {
> +            /* Sign extended 32-bit constants are formed with SETHI+XOR.  */
> +            tcg_out_sethi(s, ret, ~arg);
> +            tcg_out_arithi(s, ret, ret, (arg & 0x3ff) | -0x400, ARITH_XOR);
> +        } else {
> +            tcg_out_movi_imm32(s, TCG_REG_I4, arg >> 32);
> +            tcg_out_arithi(s, TCG_REG_I4, TCG_REG_I4, 32, SHIFT_SLLX);
> +            tcg_out_movi_imm32(s, ret, arg);
> +            tcg_out_arith(s, ret, ret, TCG_REG_I4, ARITH_OR);
> +        }
>     }
> -#else
> -    tcg_out_movi_imm32(s, ret, arg);
> -#endif
>  }

Please split this also to another patch, it looks good.


> +        int32_t val = l->u.value - (tcg_target_long)s->code_ptr;
> +        tcg_out32(s, (INSN_OP(0) | opc | INSN_OP2(0x2)
>                       | INSN_OFF22(l->u.value - (unsigned long)s->code_ptr)));
>     } else {
>         tcg_out_reloc(s, s->code_ptr, R_SPARC_WDISP22, label_index, 0);
> -        tcg_out32(s, (INSN_OP(0) | INSN_COND(opc, 0) | INSN_OP2(0x2) | 0));
> +        tcg_out32(s, (INSN_OP(0) | opc | INSN_OP2(0x2) | 0));

What instruction is this? A define would be in order.

> -        tcg_out32(s, (INSN_OP(0) | INSN_COND(opc, 0) | INSN_OP2(0x1) |
> -                      (0x5 << 19) |
> +        tcg_out32(s, (INSN_OP(0) | opc | INSN_OP2(0x1) | (0x5 << 19) |
>                       INSN_OFF19(l->u.value - (unsigned long)s->code_ptr)));
>     } else {
>         tcg_out_reloc(s, s->code_ptr, R_SPARC_WDISP19, label_index, 0);
> -        tcg_out32(s, (INSN_OP(0) | INSN_COND(opc, 0) | INSN_OP2(0x1) |
> -                      (0x5 << 19) | 0));
> +        tcg_out32(s, (INSN_OP(0) | opc | INSN_OP2(0x1) | (0x5 << 19) | 0));

Same here.

>  static void tcg_out_brcond_i32(TCGContext *s, int cond,
>                                TCGArg arg1, TCGArg arg2, int const_arg2,
>                                int label_index)
>  {
> -    if (const_arg2 && arg2 == 0)
> -        /* orcc %g0, r, %g0 */
> -        tcg_out_arith(s, TCG_REG_G0, TCG_REG_G0, arg1, ARITH_ORCC);
> -    else
> -        /* subcc r1, r2, %g0 */
> -        tcg_out_arith(s, TCG_REG_G0, arg1, arg2, ARITH_SUBCC);
> -    tcg_out_branch_i32(s, tcg_cond_to_bcond[cond], label_index);
> +    tcg_out_cmp(s, arg1, arg2, const_arg2);

What's wrong with 'orcc' (produces the synthetic instruction 'tst')?