Re: [PATCH v35 05/13] target/avr: Add instruction translation - Arithmetic and Logic Instructions

[Aleksandar Markovic]

On Tue, Nov 5, 2019 at 2:23 PM Richard Henderson
<address@hidden> wrote:
>
> On 11/5/19 10:46 AM, Aleksandar Markovic wrote:
> >
> >
> > On Tuesday, November 5, 2019, Aleksandar Markovic <address@hidden
> > <mailto:address@hidden>> wrote:
> >
> >
> >         +
> >         +/*
> >         + *  This instruction performs 8-bit x 8-bit -> 16-bit signed
> >         multiplication
> >         + *  and shifts the result one bit left.
> >         + */
> >         +static bool trans_FMULSU(DisasContext *ctx, arg_FMULSU *a)
> >         +{
> >         +    if (!avr_have_feature(ctx, AVR_FEATURE_MUL)) {
> >         +        return true;
> >         +    }
> >         +
> >         +    TCGv R0 = cpu_r[0];
> >         +    TCGv R1 = cpu_r[1];
> >         +    TCGv Rd = cpu_r[a->rd];
> >         +    TCGv Rr = cpu_r[a->rr];
> >         +    TCGv R = tcg_temp_new_i32();
> >         +    TCGv t0 = tcg_temp_new_i32();
> >         +
> >         +    tcg_gen_ext8s_tl(t0, Rd); /* make Rd full 32 bit signed */
> >         +    tcg_gen_mul_tl(R, t0, Rr); /* R = Rd * Rr */
> >         +    tcg_gen_andi_tl(R, R, 0xffff); /* make it 16 bits */
> >         +
> >         +    tcg_gen_shri_tl(cpu_Cf, R, 15); /* Cf = R(15) */
> >         +    tcg_gen_setcondi_tl(TCG_COND_EQ, cpu_Zf, R, 0); /* Zf = R == 0 
> > */
> >         +
> >         +    tcg_gen_shli_tl(R, R, 1);
> >         +
> >         +    tcg_gen_andi_tl(R0, R, 0xff);
> >         +    tcg_gen_shri_tl(R1, R, 8);
> >         +    tcg_gen_andi_tl(R1, R1, 0xff);
> >         +
> >         +    tcg_temp_free_i32(t0);
> >         +    tcg_temp_free_i32(R);
> >         +
> >         +    return true;
> >         +}
> >         +
> >
> >
> >     Hi, Michael.
> >
> >     The way I understand the spec is that a->rd and a->rd must be between 16
> >     and 23:
> >
> >     
> > https://www.microchip.com/webdoc/avrassembler/avrassembler.wb_FMULSU.html
> >     
> > <https://www.microchip.com/webdoc/avrassembler/avrassembler.wb_FMULSU.html>
> >
> >     Is my interpretation right? If yes, where is the corresponding part in 
> > the
> >     implementation?
> >
> >
> > Or, perhaps,
> >
> > TCGv Rd = cpu_r[a->rd];
> >
> > should be
> >
> > TCGv Rd = cpu_r[a->rd + 16];
> >
> > (and the same for rs)
>
> This happens during decode:
>
> +%rd_b           4:3                         !function=to_B
> +%rr_b           0:3                         !function=to_B
> +@fmul           .... .... . ... . ...       &rd_rr      rd=%rd_b rr=%rr_b
> +FMUL            0000 0011 0 ... 1 ...       @fmul
> +FMULS           0000 0011 1 ... 0 ...       @fmul
> +FMULSU          0000 0011 1 ... 1 ...       @fmul
>
> This means that a->rd = to_B(extract32(insn, 4, 3)), and
>
> > +static int to_B(DisasContext *ctx, int indx) { return 16 + (indx % 8); }
>
> et voila.
>

OK. Thanks for clarification.

>
> r~