On 12/15/20 8:01 PM, frank.ch...@sifive.com wrote: > +static bool gen_shifti(DisasContext *ctx, arg_shift *a, > + void(*func)(TCGv, TCGv, TCGv)) > +{ > + TCGv source1 = tcg_temp_new(); > + TCGv source2 = tcg_temp_new(); > + > + gen_get_gpr(source1, a->rs1); > + tcg_gen_movi_tl(source2, a->shamt); > + > + tcg_gen_andi_tl(source2, source2, TARGET_LONG_BITS - 1);
Don't make the tcg optimizer do what it's easy to do in C, e.g. tcg_gen_movi_tl(source2, a->shamt & (TARGET_LONG_BITS - 1)); Except in this case, I believe that shamt *cannot* be out of range, because that would be an illegal instruction. E.g. in trans_srli, we check for that first. In the previous review I had recommended that you convert the existing immediate shift instructions to this new interface, which should have shown this problem. > +static bool gen_shiftiw(DisasContext *ctx, arg_shift *a, > + void(*func)(TCGv, TCGv, TCGv)) > +{ > + TCGv source1 = tcg_temp_new(); > + TCGv source2 = tcg_temp_new(); > + > + gen_get_gpr(source1, a->rs1); > + tcg_gen_movi_tl(source2, a->shamt); > + > + tcg_gen_andi_tl(source2, source2, 31); This mask is unnecesary because the decode already uses @sh5. r~