All of lore.kernel.org
 help / color / mirror / Atom feed
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~


  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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.