Re: [Qemu-riscv] [Qemu-devel] [PATCH v3 24/35] target/riscv: Move gen_arith_imm() decoding into trans_* functions

[Bastian Koppelmann]

On 1/11/19 10:00 PM, Richard Henderson wrote:
> On 1/12/19 12:10 AM, Bastian Koppelmann wrote:
> > On 10/31/18 11:18 PM, Richard Henderson wrote:
> > > Surely the shri and sari functions need the same shamt >= TARGET_LONG_BITS
> > > check as slli.  Otherwise RV32 shri should definitely produce an assert in
> > > tcg_gen_shri_tl.
> > >
> > > I did wonder about changing the decode of the shift functions such that only
> > > the top two bits of the imm are reserved for secondary parsing of the shift
> > > type, and the other 10 bits are passed down into trans_foo.  At which point the
> > > TARGET_LONG_BITS check takes care of other illegalities.
> > >
> > > Which means that the parsing for slli and slliw are identical, and also that
> > > for the far future when RV128 is a thing, we don't have to change the parsing.
> >
> > I don't quite understand this. Do you want to have one entry in the decode file
> > for slli and slliw?
> >
> > How is the parsing of slli and slliw identical with this change? As far as I
> > see it, they are different at least in the opcode.
> I meant in the extraction and validation of operands, I think.
> I'm not really sure where else I was going with this.  It has
> been two months and I don't have the decode in front of me.


Okay, I extended the immediate to 10 bits for the next patch round 
(which I send out soon) which should not break anything. I suggest we 
leave this optimization for another patch as this set is already long 
and complex.

Cheers,

Bastian