Re: [Qemu-devel] [PATCH 1/6 v7] target-tilegx: Firstly add TILE-Gx with minimized features

[Richard Henderson]

On 03/20/2015 08:25 AM, Chen Gang wrote:
> +/*
> + * The related functional description for bfextu in isa document:
> + *
> + * uint64_t mask = 0;
> + * mask = (-1ULL) ^ ((-1ULL << ((BFEnd - BFStart) & 63)) << 1);
> + * uint64_t rot_src = (((uint64_t) rf[SrcA]) >> BFStart)
> + *                    | (rf[SrcA] << (64 - BFStart));
> + * rf[Dest] = rot_src & mask;
> + */
> +static void gen_bfextu(struct DisasContext *dc, uint8_t rdst, uint8_t rsrc,
> +                       int8_t start, int8_t end)
> +{
> +    uint64_t mask = (-1ULL) ^ ((-1ULL << ((end - start) & 63)) << 1);
> +    TCGv tmp = dest_gr(dc, rdst);
> +
> +    qemu_log_mask(CPU_LOG_TB_IN_ASM, "bfextu r%d, r%d, %d, %d\n",
> +                  rdst, rsrc, start, end);
> +
> +    tcg_gen_rotli_i64(tmp, load_gr(dc, rsrc), start);

Wrong direction rotate: rotri.

> +static void decode_rrr_1_opcode_y0(struct DisasContext *dc,
> +                                   tilegx_bundle_bits bundle)
> +{
> +    switch (get_RRROpcodeExtension_Y0(bundle)) {
> +    case UNARY_RRR_1_OPCODE_Y0:
> +        switch (get_UnaryOpcodeExtension_Y0(bundle)) {
> +        case NOP_UNARY_OPCODE_Y0:
> +        case  FNOP_UNARY_OPCODE_Y0:
> +            if (!get_SrcA_Y0(bundle) && !get_Dest_Y0(bundle)) {
> +                gen_fnop();
> +                return;
> +            }
> +            break;
> +        case CNTLZ_UNARY_OPCODE_Y0:
> +        case CNTTZ_UNARY_OPCODE_Y0:
> +        case FSINGLE_PACK1_UNARY_OPCODE_Y0:
> +        case PCNT_UNARY_OPCODE_Y0:
> +        case REVBITS_UNARY_OPCODE_Y0:
> +        case REVBYTES_UNARY_OPCODE_Y0:
> +        case TBLIDXB0_UNARY_OPCODE_Y0:
> +        case TBLIDXB1_UNARY_OPCODE_Y0:
> +        case TBLIDXB2_UNARY_OPCODE_Y0:
> +        case TBLIDXB3_UNARY_OPCODE_Y0:
> +        default:
> +            break;
> +        }
> +        break;
> +    case SHL1ADD_RRR_1_OPCODE_Y0:
> +    case SHL2ADD_RRR_1_OPCODE_Y0:
> +    case SHL3ADD_RRR_1_OPCODE_Y0:
> +    case RRR_1_OPCODE_Y0:

RRR_1_OPCODE_Y0 doesn't belong.  It's the main opcode that brought us here.

> +    default:
> +        break;
> +    }
> +
> +    qemu_log_mask(LOG_UNIMP, "UNIMP rrr_1_opcode_y0, [" TARGET_FMT_lx "]\n",
> +                  (uint64_t)bundle);

Eh, TARGET_FMT_lx is tied to TARGET_LONG_BITS, and you're not using target_long
but uint64_t.  I think it would be better to use "%016" PRIx64 directly, or
create your own macro for this file.


r~