Re: [Qemu-devel] [PATCH 04/28] target/riscv: Convert RVXI arithmetic insns to decodetree

[Palmer Dabbelt]

On Fri, 19 Oct 2018 04:00:33 PDT (-0700), address@hidden wrote:
> Hi Richard,
>
> On 10/12/18 8:46 PM, Richard Henderson wrote:
> > On 10/12/18 10:30 AM, Bastian Koppelmann wrote:
> > > +static bool trans_andi(DisasContext *ctx, arg_andi *a, uint32_t insn)
> > > +{
> > > +    gen_arith_imm(ctx, OPC_RISC_ANDI, a->rd, a->rs1, a->imm);
> > > +    return true;
> > > +}
> > > +static bool trans_slli(DisasContext *ctx, arg_slli *a, uint32_t insn)
> > > +{
> > > +    if (a->rd != 0) {
> > > +        TCGv t = tcg_temp_new();
> > > +        gen_get_gpr(t, a->rs1);
> > > +
> > > +        if (a->shamt >= TARGET_LONG_BITS) {
> > > +            gen_exception_illegal(ctx);
> > > +            return true;
> > > +        }
> > > +        tcg_gen_shli_tl(t, t, a->shamt);
> > > +
> > > +        gen_set_gpr(a->rd, t);
> > > +        tcg_temp_free(t);
> > > +    } /* NOP otherwise */
> > > +    return true;
> > > +}
> > Spacing.  Any reason why trans_slli (and the other shifts) aren't using
> > gen_arith_imm as well?
>
>
> Their opcode is not uniquely defined in instmap.h, just a generic
> OPC_RISC_SHIFT_RIGHT_IW. I guess I can give the opcode as a magic value
> for now.

Shifts are the only arithmetic operations that aren't uniquely defined by func3 
(the opcode), but instead have another bit packed in where the immediate 
usually lives to differentiate between arithmetic and logical shifts.  This 
pretty much always ends up as a bit of a special case in software decoders.