qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 1/2] target/riscv: Quiet Coverity complains about vamo*
@ 2020-07-21 13:37 LIU Zhiwei
  2020-07-21 13:37 ` [PATCH 2/2] target/riscv: fix vector index load/store constraints LIU Zhiwei
  2020-07-21 15:07 ` [PATCH 1/2] target/riscv: Quiet Coverity complains about vamo* Alistair Francis
  0 siblings, 2 replies; 6+ messages in thread
From: LIU Zhiwei @ 2020-07-21 13:37 UTC (permalink / raw)
  To: qemu-devel, qemu-riscv; +Cc: Alistair.Francis, richard.henderson, LIU Zhiwei

Signed-off-by: LIU Zhiwei <zhiwei_liu@c-sky.com>
---
 target/riscv/insn_trans/trans_rvv.inc.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/target/riscv/insn_trans/trans_rvv.inc.c b/target/riscv/insn_trans/trans_rvv.inc.c
index c0b7375927..7b4752b911 100644
--- a/target/riscv/insn_trans/trans_rvv.inc.c
+++ b/target/riscv/insn_trans/trans_rvv.inc.c
@@ -733,6 +733,7 @@ static bool amo_op(DisasContext *s, arg_rwdvm *a, uint8_t seq)
             g_assert_not_reached();
 #endif
         } else {
+            assert(seq < ARRAY_SIZE(fnsw));
             fn = fnsw[seq];
         }
     }
-- 
2.23.0



^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [PATCH 2/2] target/riscv: fix vector index load/store constraints
  2020-07-21 13:37 [PATCH 1/2] target/riscv: Quiet Coverity complains about vamo* LIU Zhiwei
@ 2020-07-21 13:37 ` LIU Zhiwei
  2020-07-21 15:11   ` Alistair Francis
  2020-07-21 15:07 ` [PATCH 1/2] target/riscv: Quiet Coverity complains about vamo* Alistair Francis
  1 sibling, 1 reply; 6+ messages in thread
From: LIU Zhiwei @ 2020-07-21 13:37 UTC (permalink / raw)
  To: qemu-devel, qemu-riscv; +Cc: Alistair.Francis, richard.henderson, LIU Zhiwei

Although not explicitly specified that the the destination
vector register groups cannot overlap the source vector register group,
it is still necessary.

And this constraint has been added to the v0.8 spec.

Signed-off-by: LIU Zhiwei <zhiwei_liu@c-sky.com>
---
 target/riscv/insn_trans/trans_rvv.inc.c | 10 +++++++++-
 1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/target/riscv/insn_trans/trans_rvv.inc.c b/target/riscv/insn_trans/trans_rvv.inc.c
index 7b4752b911..887c6b8883 100644
--- a/target/riscv/insn_trans/trans_rvv.inc.c
+++ b/target/riscv/insn_trans/trans_rvv.inc.c
@@ -513,13 +513,21 @@ static bool ld_index_op(DisasContext *s, arg_rnfvm *a, uint8_t seq)
     return ldst_index_trans(a->rd, a->rs1, a->rs2, data, fn, s);
 }
 
+/*
+ * For vector indexed segment loads, the destination vector register
+ * groups cannot overlap the source vector register group (specified by
+ * `vs2`), else an illegal instruction exception is raised.
+ */
 static bool ld_index_check(DisasContext *s, arg_rnfvm* a)
 {
     return (vext_check_isa_ill(s) &&
             vext_check_overlap_mask(s, a->rd, a->vm, false) &&
             vext_check_reg(s, a->rd, false) &&
             vext_check_reg(s, a->rs2, false) &&
-            vext_check_nf(s, a->nf));
+            vext_check_nf(s, a->nf) &&
+            ((a->nf == 1) ||
+             vext_check_overlap_group(a->rd, a->nf << s->lmul,
+                                      a->rs2, 1 << s->lmul)));
 }
 
 GEN_VEXT_TRANS(vlxb_v, 0, rnfvm, ld_index_op, ld_index_check)
-- 
2.23.0



^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH 1/2] target/riscv: Quiet Coverity complains about vamo*
  2020-07-21 13:37 [PATCH 1/2] target/riscv: Quiet Coverity complains about vamo* LIU Zhiwei
  2020-07-21 13:37 ` [PATCH 2/2] target/riscv: fix vector index load/store constraints LIU Zhiwei
@ 2020-07-21 15:07 ` Alistair Francis
  2020-07-21 15:30   ` Peter Maydell
  1 sibling, 1 reply; 6+ messages in thread
From: Alistair Francis @ 2020-07-21 15:07 UTC (permalink / raw)
  To: LIU Zhiwei
  Cc: Alistair Francis, Richard Henderson, open list:RISC-V,
	qemu-devel@nongnu.org Developers

On Tue, Jul 21, 2020 at 6:38 AM LIU Zhiwei <zhiwei_liu@c-sky.com> wrote:
>
> Signed-off-by: LIU Zhiwei <zhiwei_liu@c-sky.com>

Reviewed-by: Alistair Francis <alistair.francis@wdc.com>

Alistair

> ---
>  target/riscv/insn_trans/trans_rvv.inc.c | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/target/riscv/insn_trans/trans_rvv.inc.c b/target/riscv/insn_trans/trans_rvv.inc.c
> index c0b7375927..7b4752b911 100644
> --- a/target/riscv/insn_trans/trans_rvv.inc.c
> +++ b/target/riscv/insn_trans/trans_rvv.inc.c
> @@ -733,6 +733,7 @@ static bool amo_op(DisasContext *s, arg_rwdvm *a, uint8_t seq)
>              g_assert_not_reached();
>  #endif
>          } else {
> +            assert(seq < ARRAY_SIZE(fnsw));
>              fn = fnsw[seq];
>          }
>      }
> --
> 2.23.0
>
>


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH 2/2] target/riscv: fix vector index load/store constraints
  2020-07-21 13:37 ` [PATCH 2/2] target/riscv: fix vector index load/store constraints LIU Zhiwei
@ 2020-07-21 15:11   ` Alistair Francis
  0 siblings, 0 replies; 6+ messages in thread
From: Alistair Francis @ 2020-07-21 15:11 UTC (permalink / raw)
  To: LIU Zhiwei
  Cc: Alistair Francis, Richard Henderson, open list:RISC-V,
	qemu-devel@nongnu.org Developers

On Tue, Jul 21, 2020 at 6:38 AM LIU Zhiwei <zhiwei_liu@c-sky.com> wrote:
>
> Although not explicitly specified that the the destination
> vector register groups cannot overlap the source vector register group,
> it is still necessary.
>
> And this constraint has been added to the v0.8 spec.
>
> Signed-off-by: LIU Zhiwei <zhiwei_liu@c-sky.com>

Reviewed-by: Alistair Francis <alistair.francis@wdc.com>

Alistair

> ---
>  target/riscv/insn_trans/trans_rvv.inc.c | 10 +++++++++-
>  1 file changed, 9 insertions(+), 1 deletion(-)
>
> diff --git a/target/riscv/insn_trans/trans_rvv.inc.c b/target/riscv/insn_trans/trans_rvv.inc.c
> index 7b4752b911..887c6b8883 100644
> --- a/target/riscv/insn_trans/trans_rvv.inc.c
> +++ b/target/riscv/insn_trans/trans_rvv.inc.c
> @@ -513,13 +513,21 @@ static bool ld_index_op(DisasContext *s, arg_rnfvm *a, uint8_t seq)
>      return ldst_index_trans(a->rd, a->rs1, a->rs2, data, fn, s);
>  }
>
> +/*
> + * For vector indexed segment loads, the destination vector register
> + * groups cannot overlap the source vector register group (specified by
> + * `vs2`), else an illegal instruction exception is raised.
> + */
>  static bool ld_index_check(DisasContext *s, arg_rnfvm* a)
>  {
>      return (vext_check_isa_ill(s) &&
>              vext_check_overlap_mask(s, a->rd, a->vm, false) &&
>              vext_check_reg(s, a->rd, false) &&
>              vext_check_reg(s, a->rs2, false) &&
> -            vext_check_nf(s, a->nf));
> +            vext_check_nf(s, a->nf) &&
> +            ((a->nf == 1) ||
> +             vext_check_overlap_group(a->rd, a->nf << s->lmul,
> +                                      a->rs2, 1 << s->lmul)));
>  }
>
>  GEN_VEXT_TRANS(vlxb_v, 0, rnfvm, ld_index_op, ld_index_check)
> --
> 2.23.0
>
>


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH 1/2] target/riscv: Quiet Coverity complains about vamo*
  2020-07-21 15:07 ` [PATCH 1/2] target/riscv: Quiet Coverity complains about vamo* Alistair Francis
@ 2020-07-21 15:30   ` Peter Maydell
  2020-07-22 16:40     ` Alistair Francis
  0 siblings, 1 reply; 6+ messages in thread
From: Peter Maydell @ 2020-07-21 15:30 UTC (permalink / raw)
  To: Alistair Francis
  Cc: Richard Henderson, Alistair Francis, LIU Zhiwei,
	open list:RISC-V, qemu-devel@nongnu.org Developers

On Tue, 21 Jul 2020 at 16:19, Alistair Francis <alistair23@gmail.com> wrote:
>
> On Tue, Jul 21, 2020 at 6:38 AM LIU Zhiwei <zhiwei_liu@c-sky.com> wrote:
> >
> > Signed-off-by: LIU Zhiwei <zhiwei_liu@c-sky.com>
>
> Reviewed-by: Alistair Francis <alistair.francis@wdc.com>

Interestingly Coverity's latest scan has decided all these
issues are 'fixed', even without the assert. I guess that the
online version of Coverity has had an improvement to its
checking and so is now able to figure out that it's not going
to overrun the array? Still I think the assert is worth having.

thanks
-- PMM


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH 1/2] target/riscv: Quiet Coverity complains about vamo*
  2020-07-21 15:30   ` Peter Maydell
@ 2020-07-22 16:40     ` Alistair Francis
  0 siblings, 0 replies; 6+ messages in thread
From: Alistair Francis @ 2020-07-22 16:40 UTC (permalink / raw)
  To: Peter Maydell
  Cc: Richard Henderson, Alistair Francis, LIU Zhiwei,
	open list:RISC-V, qemu-devel@nongnu.org Developers

On Tue, Jul 21, 2020 at 8:30 AM Peter Maydell <peter.maydell@linaro.org> wrote:
>
> On Tue, 21 Jul 2020 at 16:19, Alistair Francis <alistair23@gmail.com> wrote:
> >
> > On Tue, Jul 21, 2020 at 6:38 AM LIU Zhiwei <zhiwei_liu@c-sky.com> wrote:
> > >
> > > Signed-off-by: LIU Zhiwei <zhiwei_liu@c-sky.com>
> >
> > Reviewed-by: Alistair Francis <alistair.francis@wdc.com>
>
> Interestingly Coverity's latest scan has decided all these
> issues are 'fixed', even without the assert. I guess that the
> online version of Coverity has had an improvement to its
> checking and so is now able to figure out that it's not going
> to overrun the array? Still I think the assert is worth having.

Strange.

I'll still apply these two patches.

Alistair

>
> thanks
> -- PMM


^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2020-07-22 16:51 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-07-21 13:37 [PATCH 1/2] target/riscv: Quiet Coverity complains about vamo* LIU Zhiwei
2020-07-21 13:37 ` [PATCH 2/2] target/riscv: fix vector index load/store constraints LIU Zhiwei
2020-07-21 15:11   ` Alistair Francis
2020-07-21 15:07 ` [PATCH 1/2] target/riscv: Quiet Coverity complains about vamo* Alistair Francis
2020-07-21 15:30   ` Peter Maydell
2020-07-22 16:40     ` Alistair Francis

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).