1. Patchew
  2. QEMU
  3. View series

[PATCH] tcg/riscv64: Set vtype before whole-register vector loads

Richard Henderson posted 1 patch 1 week ago 
tcg/riscv64/tcg-target.c.inc | 6 ++++++
1 file changed, 6 insertions(+)
[PATCH] tcg/riscv64: Set vtype before whole-register vector loads
Posted by Richard Henderson 1 week ago 
With vtype uninitialized, VILL might be set, leading to SIGILL.
Also, assert vtype initialized for whole-register stores and moves.

Cc: Palmer Dabbelt <palmer@dabbelt.com>
Reported-by: Max Chou <max.chou@sifive.com>
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
 tcg/riscv64/tcg-target.c.inc | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/tcg/riscv64/tcg-target.c.inc b/tcg/riscv64/tcg-target.c.inc
index 2ce9d47a633..de9a3570b23 100644
--- a/tcg/riscv64/tcg-target.c.inc
+++ b/tcg/riscv64/tcg-target.c.inc
@@ -775,6 +775,7 @@ static bool tcg_out_mov(TCGContext *s, TCGType type, TCGReg ret, TCGReg arg)
     case TCG_TYPE_V64:
     case TCG_TYPE_V128:
     case TCG_TYPE_V256:
+        tcg_debug_assert(s->riscv_cur_type != TCG_TYPE_COUNT);
         {
             int lmul = type - riscv_lg2_vlenb;
             int nf = 1 << MAX(lmul, 0);
@@ -1014,6 +1015,10 @@ static void tcg_out_ld(TCGContext *s, TCGType type, TCGReg arg,
             unsigned idx = type - riscv_lg2_vlenb;
 
             tcg_debug_assert(idx < ARRAY_SIZE(whole_reg_ld));
+            /* We must initialize vtype to something to avoid VILL. */
+            if (s->riscv_cur_type == TCG_TYPE_COUNT) {
+                set_vtype(s, type, MO_8);
+            }
             insn = whole_reg_ld[idx];
         } else {
             static const RISCVInsn unit_stride_ld[] = {
@@ -1046,6 +1051,7 @@ static void tcg_out_st(TCGContext *s, TCGType type, TCGReg arg,
     case TCG_TYPE_V64:
     case TCG_TYPE_V128:
     case TCG_TYPE_V256:
+        tcg_debug_assert(s->riscv_cur_type != TCG_TYPE_COUNT);
         if (type >= riscv_lg2_vlenb) {
             static const RISCVInsn whole_reg_st[] = {
                 OPC_VS1R_V, OPC_VS2R_V, OPC_VS4R_V, OPC_VS8R_V
-- 
2.53.0
Re: [PATCH] tcg/riscv64: Set vtype before whole-register vector loads
Posted by Michael Tokarev 2 days, 16 hours ago 
On 9/18/26 23:10, Richard Henderson wrote:
> With vtype uninitialized, VILL might be set, leading to SIGILL.
> Also, assert vtype initialized for whole-register stores and moves.
> 
> Cc: Palmer Dabbelt <palmer@dabbelt.com>
> Reported-by: Max Chou <max.chou@sifive.com>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>

This looks like a qemu-stable material.
I'm picking it up, please let me know if I shouldn't.

Thanks,

/mjt
Re: [PATCH] tcg/riscv64: Set vtype before whole-register vector loads
Posted by Palmer Dabbelt 1 week ago 
On Fri, 18 Sep 2026 13:10:07 PDT (-0700), Richard Henderson wrote:
> With vtype uninitialized, VILL might be set, leading to SIGILL.
> Also, assert vtype initialized for whole-register stores and moves.
>
> Cc: Palmer Dabbelt <palmer@dabbelt.com>
> Reported-by: Max Chou <max.chou@sifive.com>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
>  tcg/riscv64/tcg-target.c.inc | 6 ++++++
>  1 file changed, 6 insertions(+)
>
> diff --git a/tcg/riscv64/tcg-target.c.inc b/tcg/riscv64/tcg-target.c.inc
> index 2ce9d47a633..de9a3570b23 100644
> --- a/tcg/riscv64/tcg-target.c.inc
> +++ b/tcg/riscv64/tcg-target.c.inc
> @@ -775,6 +775,7 @@ static bool tcg_out_mov(TCGContext *s, TCGType type, TCGReg ret, TCGReg arg)
>      case TCG_TYPE_V64:
>      case TCG_TYPE_V128:
>      case TCG_TYPE_V256:
> +        tcg_debug_assert(s->riscv_cur_type != TCG_TYPE_COUNT);
>          {
>              int lmul = type - riscv_lg2_vlenb;
>              int nf = 1 << MAX(lmul, 0);
> @@ -1014,6 +1015,10 @@ static void tcg_out_ld(TCGContext *s, TCGType type, TCGReg arg,
>              unsigned idx = type - riscv_lg2_vlenb;
>
>              tcg_debug_assert(idx < ARRAY_SIZE(whole_reg_ld));
> +            /* We must initialize vtype to something to avoid VILL. */
> +            if (s->riscv_cur_type == TCG_TYPE_COUNT) {
> +                set_vtype(s, type, MO_8);
> +            }
>              insn = whole_reg_ld[idx];
>          } else {
>              static const RISCVInsn unit_stride_ld[] = {
> @@ -1046,6 +1051,7 @@ static void tcg_out_st(TCGContext *s, TCGType type, TCGReg arg,
>      case TCG_TYPE_V64:
>      case TCG_TYPE_V128:
>      case TCG_TYPE_V256:
> +        tcg_debug_assert(s->riscv_cur_type != TCG_TYPE_COUNT);
>          if (type >= riscv_lg2_vlenb) {
              static const RISCVInsn whole_reg_st[] = {
>                  OPC_VS1R_V, OPC_VS2R_V, OPC_VS4R_V, OPC_VS8R_V

Thanks!  Looks like it was just a lurking bug then.

Reviewed-by: Palmer Dabbelt <palmer@dabbelt.com>

and

Fixes: f63e7089b4 ("tcg/riscv: Add basic support for vector")

though that's old enough I'm not sure it really matters.

It looks like we're clobbering vtype and the rest of the V registers on calls,
so I think we're good there.

I poked through and it looks like every other case that constructs a V
instruction that depends on the vector configuration has a vtype set
immediately before it, so I think the rest of the port is safe.  That said, if
I understand the ISA correctly, we need a valid vector configuration before
executing any V instruction (assuming the post-change ISA), so I think we could
also just throw something like this in there and maybe catch some mistakes in
the future?

diff --git a/tcg/riscv64/tcg-target.c.inc b/tcg/riscv64/tcg-target.c.inc
index 2ce9d47a63..42fcc8182e 100644
--- a/tcg/riscv64/tcg-target.c.inc
+++ b/tcg/riscv64/tcg-target.c.inc
@@ -686,18 +686,21 @@ static bool patch_reloc(tcg_insn_unit *code_ptr, int type,
 static void tcg_out_opc_vv(TCGContext *s, RISCVInsn opc,
                            TCGReg vd, TCGReg vs2, TCGReg vs1)
 {
+    tcg_debug_assert(s->riscv_cur_type != TCG_TYPE_COUNT);
     tcg_out32(s, encode_v(opc, vd, vs1, vs2, true));
 }
 
 static void tcg_out_opc_vx(TCGContext *s, RISCVInsn opc,
                            TCGReg vd, TCGReg vs2, TCGReg rs1)
 {
+    tcg_debug_assert(s->riscv_cur_type != TCG_TYPE_COUNT);
     tcg_out32(s, encode_v(opc, vd, rs1, vs2, true));
 }
 
 static void tcg_out_opc_vi(TCGContext *s, RISCVInsn opc,
                            TCGReg vd, TCGReg vs2, int32_t imm)
 {
+    tcg_debug_assert(s->riscv_cur_type != TCG_TYPE_COUNT);
     tcg_out32(s, encode_vi(opc, vd, imm, vs2, true));
 }
 
@@ -714,12 +717,14 @@ static void tcg_out_opc_vv_vi(TCGContext *s, RISCVInsn o_vv, RISCVInsn o_vi,
 static void tcg_out_opc_vim_mask(TCGContext *s, RISCVInsn opc, TCGReg vd,
                                  TCGReg vs2, int32_t imm)
 {
+    tcg_debug_assert(s->riscv_cur_type != TCG_TYPE_COUNT);
     tcg_out32(s, encode_vi(opc, vd, imm, vs2, false));
 }
 
 static void tcg_out_opc_vvm_mask(TCGContext *s, RISCVInsn opc, TCGReg vd,
                                  TCGReg vs2, TCGReg vs1)
 {
+    tcg_debug_assert(s->riscv_cur_type != TCG_TYPE_COUNT);
     tcg_out32(s, encode_v(opc, vd, vs1, vs2, false));
 }
 
@@ -978,6 +983,7 @@ static void tcg_out_vec_ldst(TCGContext *s, RISCVInsn opc, TCGReg data,
 {
     tcg_debug_assert(data >= TCG_REG_V0);
     tcg_debug_assert(addr < TCG_REG_V0);
+    tcg_debug_assert(s->riscv_cur_type != TCG_TYPE_COUNT);
 
     if (offset) {
         tcg_debug_assert(addr != TCG_REG_ZERO);
@@ -1554,6 +1560,7 @@ static void tcg_out_cmpsel(TCGContext *s, TCGType type, unsigned vece,
 static void tcg_out_vshifti(TCGContext *s, RISCVInsn opc_vi, RISCVInsn opc_vx,
                              TCGReg dst, TCGReg src, unsigned imm)
 {
+    tcg_debug_assert(s->riscv_cur_type != TCG_TYPE_COUNT);
     if (imm < 32) {
         tcg_out_opc_vi(s, opc_vi, dst, src, imm);
     } else {
Re: [PATCH] tcg/riscv64: Set vtype before whole-register vector loads
Posted by Richard Henderson 1 week ago 
On 9/18/26 12:38, Palmer Dabbelt wrote:
> That said, if I understand the ISA correctly, we need a valid vector
> configuration before executing any V instruction (assuming the post-
> change ISA), so I think we could also just throw something like this
> in there and maybe catch some mistakes in the future?
Yes, that seems reasonable.

While you're at it, can I get you to look at

https://patchew.org/QEMU/20260914000814.785155-1-richard.henderson@linaro.org/

which does something similar in adding assertions against the integer 
instruction formats.


r~

Patchew 2.1-devel-ea86937

© 2016 - 2026 Red Hat, Inc.