From: Alistair Francis <alistair23@gmail.com>
To: Richard Henderson <richard.henderson@linaro.org>
Cc: Chih-Min Chao <chihmin.chao@sifive.com>,
Alistair Francis <alistair.francis@wdc.com>,
LIU Zhiwei <zhiwei_liu@c-sky.com>,
"qemu-devel@nongnu.org Developers" <qemu-devel@nongnu.org>
Subject: Re: [PULL 05/20] target/riscv: Check nanboxed inputs in trans_rvf.inc.c
Date: Thu, 13 Aug 2020 14:19:57 -0700 [thread overview]
Message-ID: <CAKmqyKNKwUixJBKfKs6EedN0ZxpZ-SCQduyvbKb-gf1R=4odUg@mail.gmail.com> (raw)
In-Reply-To: <dca8e8bb-5f30-84e1-c2a1-02b281c24c79@linaro.org>
On Thu, Aug 13, 2020 at 9:48 AM Richard Henderson
<richard.henderson@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~
next prev parent reply other threads:[~2020-08-13 21:31 UTC|newest]
Thread overview: 26+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-08-12 22:30 [PULL 00/20] riscv-to-apply queue Alistair Francis
2020-08-12 22:30 ` [PULL 01/20] target/riscv: Generate nanboxed results from fp helpers Alistair Francis
2020-08-12 22:30 ` [PULL 02/20] target/riscv: Generalize gen_nanbox_fpr to gen_nanbox_s Alistair Francis
2020-08-12 22:30 ` [PULL 03/20] target/riscv: Generate nanboxed results from trans_rvf.inc.c Alistair Francis
2020-08-12 22:30 ` [PULL 04/20] target/riscv: Check nanboxed inputs to fp helpers Alistair Francis
2020-08-12 22:30 ` [PULL 05/20] target/riscv: Check nanboxed inputs in trans_rvf.inc.c Alistair Francis
2020-08-13 2:14 ` LIU Zhiwei
2020-08-13 14:46 ` Alistair Francis
2020-08-13 16:48 ` Richard Henderson
2020-08-13 21:19 ` Alistair Francis [this message]
2020-08-12 22:30 ` [PULL 06/20] target/riscv: Clean up fmv.w.x Alistair Francis
2020-08-12 22:30 ` [PULL 07/20] target/riscv: check before allocating TCG temps Alistair Francis
2020-08-12 22:30 ` [PULL 08/20] hw/riscv: sifive_u: Add a dummy L2 cache controller device Alistair Francis
2020-08-12 22:30 ` [PULL 09/20] riscv: Fix bug in setting pmpcfg CSR for RISCV64 Alistair Francis
2020-08-12 22:30 ` [PULL 10/20] configure: Create symbolic links for pc-bios/*.elf files Alistair Francis
2020-08-12 22:30 ` [PULL 11/20] roms/opensbi: Upgrade from v0.7 to v0.8 Alistair Francis
2020-08-12 22:30 ` [PULL 12/20] roms/Makefile: Build the generic platform for RISC-V OpenSBI firmware Alistair Francis
2020-08-12 22:30 ` [PULL 13/20] hw/riscv: Use pre-built bios image of generic platform for virt & sifive_u Alistair Francis
2020-08-12 22:30 ` [PULL 14/20] hw/riscv: spike: Change the default bios to use generic platform image Alistair Francis
2020-08-12 22:30 ` [PULL 15/20] gitlab-ci/opensbi: Update GitLab CI to build generic platform Alistair Francis
2020-08-12 22:30 ` [PULL 16/20] target/riscv: Fix the translation of physical address Alistair Francis
2020-08-12 22:30 ` [PULL 17/20] target/riscv: Change the TLB page size depends on PMP entries Alistair Francis
2020-08-12 22:30 ` [PULL 18/20] hw/intc: ibex_plic: Update the pending irqs Alistair Francis
2020-08-12 22:30 ` [PULL 19/20] hw/intc: ibex_plic: Don't allow repeat interrupts on claimed lines Alistair Francis
2020-08-12 22:30 ` [PULL 20/20] hw/intc: ibex_plic: Honour source priorities Alistair Francis
2020-08-13 7:53 ` [PULL 00/20] riscv-to-apply queue Philippe Mathieu-Daudé
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to='CAKmqyKNKwUixJBKfKs6EedN0ZxpZ-SCQduyvbKb-gf1R=4odUg@mail.gmail.com' \
--to=alistair23@gmail.com \
--cc=alistair.francis@wdc.com \
--cc=chihmin.chao@sifive.com \
--cc=qemu-devel@nongnu.org \
--cc=richard.henderson@linaro.org \
--cc=zhiwei_liu@c-sky.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).