On Thu, Aug 13, 2020 at 9:48 AM Richard Henderson <richard.hender...@linaro.org> wrote: > > On 8/13/20 7:46 AM, Alistair Francis wrote: > >> Hi Alistair, > >> > >> As Chih-Min said, it's wrong here. He has given the correct patch code > >> https://www.mail-archive.com/qemu-devel@nongnu.org/msg728540.html > >> > >> We can either squash the code to this patch or add an separate patch > >> later. I prefer the former. > >> Thanks very much. > > > > Richard are you ok if I squash this diff into the patch and send a PR v2? > > > > diff --git a/target/riscv/insn_trans/trans_rvf.inc.c > > b/target/riscv/insn_trans/trans_rvf.inc.c > > index f9a9e0643a..76f281d275 100644 > > --- a/target/riscv/insn_trans/trans_rvf.inc.c > > +++ b/target/riscv/insn_trans/trans_rvf.inc.c > > @@ -201,7 +201,8 @@ static bool trans_fsgnjn_s(DisasContext *ctx, > > arg_fsgnjn_s *a) > > * This formulation retains the nanboxing of rs1. > > */ > > mask = tcg_const_i64(~MAKE_64BIT_MASK(31, 1)); > > - tcg_gen_andc_i64(rs2, mask, rs2); > > + tcg_gen_not_i64(rs2, rs2); // forget to inverse rs2 > > + tcg_gen_andc_i64(rs2, rs2, mask); //mask needs to be > > Ah, well. Yes, it's a bug. However, > > ~rs2 & ~mask > = ~(rs2 | mask) > > so a better fix could be > > - tcg_gen_andc_i64(rs2, mask, rs2); > + tcg_gen_nor_i64(rs2, rs2, mask);
Fixed. Alistair > > > As an aside, I think perhaps I should have added a ppc-style rotate-and-insert > primitive to handle this sort of bitfield insert, since the best set of host > insns to perform this operation, when the start of the field is not bit 0, is > difficult to predict from the translator. > > > r~