qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PULL 05/20] target/riscv: Check nanboxed inputs in trans_rvf.inc.c


From: Richard Henderson
Subject: Re: [PULL 05/20] target/riscv: Check nanboxed inputs in trans_rvf.inc.c
Date: Thu, 13 Aug 2020 09:48:52 -0700
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101 Thunderbird/68.10.0

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~



reply via email to

[Prev in Thread] Current Thread [Next in Thread]