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); 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~
