[PATCH v3 0/5] disas/riscv: Fix immediate decoding and extraction

All of lore.kernel.org
 help / color / mirror / Atom feed

* [PATCH v3 0/5] disas/riscv: Fix immediate decoding and extraction
@ 2026-07-01 14:32 TANG Tiancheng
  2026-07-01 14:32 ` [PATCH v3 1/5] disas/riscv: Fix th.srri decoding TANG Tiancheng
                   ` (4 more replies)
  0 siblings, 5 replies; 9+ messages in thread
From: TANG Tiancheng @ 2026-07-01 14:32 UTC (permalink / raw)
  To: qemu-devel
  Cc: Christoph Muellner, LIU Zhiwei, Palmer Dabbelt, Alistair Francis,
	Daniel Henrique Barboza, Richard Henderson, qemu-riscv,
	TANG Tiancheng

Fix two XThead immediate decoding issues in the RISC-V disassembler, fix
vector immediate signedness handling, and make operand extraction more
explicit.

The first patch aligns th.srri decoding with target/riscv/xthead.decode:
th_srri uses bits 31:26 as 000100 and bit 25 as shamt[5], while the
disassembler switches on bits 31:25.

The second patch extracts the full 6-bit immediate field for
rv_codec_r2_imm6, which fixes the immediate printed for th.srri and
th.tst.

The third patch makes operand_vimm() return int32_t, matching the signed
5-bit vector immediate value it extracts.

The fourth patch decodes unsigned 5-bit vector immediates with a separate
codec, matching the target translator paths that zero-extend or truncate
those fields.

The fifth patch follows Richard's review suggestion by converting operand
helper bit extraction to extract32() or sextract32(). Signed immediates use
sextract32() on the sign-bearing field and combine it with the remaining
extract32() fields.

This series is based on Alistair's riscv-to-apply.next branch:
9afc75682112 ("hw/riscv/riscv-iommu.c: make FCTL.BE read only 0")

Testing:
- Applied and tested on top of Alistair's riscv-to-apply.next branch.
- Verified with an external test branch that adds unit coverage:
  https://github.com/LLyndra/qemu.git b4-disas-riscv-v2-tests

  The added tests cover XThead th.srri/th.tst immediate decoding,
  signed vector immediates, unsigned vector immediate operands, and
  signed base ISA I-immediate extraction.
- Commands run:
    ./configure --target-list=riscv64-softmmu --disable-docs \
        --prefix=/tmp/qemu-riscv-disas-test --cross-prefix=
    ninja -C build tests/unit/test-riscv-disas
    build/tests/unit/test-riscv-disas --tap -k
- Result:
    test-riscv-disas passed: 37/37 TAP tests ok.

---
Changes in v3:
- Updated the extract-helper cleanup patch to use sextract32() on the
  sign-bearing field directly, as suggested by Richard.
- Link to v2: https://lore.kernel.org/qemu-devel/20260701-b4-disas-xthead-fix-riscv-next-v2-0-e6e4ff5a7847@linux.alibaba.com

Changes in v2:
- Collected Reviewed-by tags from Daniel.
- Kept the 6-bit immediate fix as a minimal correctness change.
- Added a patch making the signed vector immediate helper return a signed
  type.
- Added a patch decoding unsigned vector immediates separately from signed
  vector immediates.
- Added a cleanup patch using extract32()/sextract32() for replaceable
  operand helper field extraction, including split immediates, as
  suggested by Richard.
- Link to v1: https://lore.kernel.org/qemu-devel/20260626-b4-disas-xthead-fix-riscv-next-v1-0-74228a3c1b21@linux.alibaba.com

---
TANG Tiancheng (5):
      disas/riscv: Fix th.srri decoding
      disas/riscv: Fix 6-bit immediate extraction
      disas/riscv: Use signed type for vector immediates
      disas/riscv: Decode unsigned vector immediates as unsigned
      disas/riscv: Use extract helpers for operand fields

 disas/riscv-xthead.c |   6 +-
 disas/riscv.c        | 263 +++++++++++++++++++++++++++------------------------
 disas/riscv.h        |   1 +
 3 files changed, 142 insertions(+), 128 deletions(-)
---
base-commit: 9afc756821123d0b9dfbf3c5673d82b048a6248a
change-id: 20260623-b4-disas-xthead-fix-riscv-next-c6af6a09d2dc

Best regards,
-- 
TANG Tiancheng <lyndra@linux.alibaba.com>



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

* [PATCH v3 1/5] disas/riscv: Fix th.srri decoding
  2026-07-01 14:32 [PATCH v3 0/5] disas/riscv: Fix immediate decoding and extraction TANG Tiancheng
@ 2026-07-01 14:32 ` TANG Tiancheng
  2026-07-01 14:32 ` [PATCH v3 2/5] disas/riscv: Fix 6-bit immediate extraction TANG Tiancheng
                   ` (3 subsequent siblings)
  4 siblings, 0 replies; 9+ messages in thread
From: TANG Tiancheng @ 2026-07-01 14:32 UTC (permalink / raw)
  To: qemu-devel
  Cc: Christoph Muellner, LIU Zhiwei, Palmer Dabbelt, Alistair Francis,
	Daniel Henrique Barboza, Richard Henderson, qemu-riscv,
	TANG Tiancheng

target/riscv/xthead.decode defines th.srri as funct6=000100 in
bits 31:26, with the 6-bit immediate in bits 25:20.

decode_xtheadbb() switches on bits 31:25, i.e. funct6 plus imm[5].
Therefore valid th.srri encodings are 0001000 and 0001001. The
current 0000100 and 0000101 cases use the wrong funct6 value and
decode valid th.srri instructions as illegal.

Fix the cases to match funct6=000100 with both imm[5] values.

Fixes: 318df7238b9f ("disas/riscv: Add support for XThead* instructions")
Signed-off-by: TANG Tiancheng <lyndra@linux.alibaba.com>
Reviewed-by: LIU Zhiwei <zhiwei_liu@linux.alibaba.com>
Reviewed-by: Daniel Henrique Barboza <daniel.barboza@oss.qualcomm.com>
---
 disas/riscv-xthead.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/disas/riscv-xthead.c b/disas/riscv-xthead.c
index fcca326d1c398a6745984b9f5d90fc5f7d54dfd9..2f2ddb8dd9a13e17a28539b4179cd10fa93c5941 100644
--- a/disas/riscv-xthead.c
+++ b/disas/riscv-xthead.c
@@ -315,8 +315,10 @@ void decode_xtheadbb(rv_decode *dec, rv_isa isa)
                         op = rv_op_th_revw;
                     }
                     break;
-                case 0b0000100:
-                case 0b0000101: op = rv_op_th_srri; break;
+                case 0b0001000:
+                case 0b0001001:
+                    op = rv_op_th_srri;
+                    break;
                 }
                 break;
             case 2: op = rv_op_th_ext; break;

-- 
2.43.0



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

* [PATCH v3 2/5] disas/riscv: Fix 6-bit immediate extraction
  2026-07-01 14:32 [PATCH v3 0/5] disas/riscv: Fix immediate decoding and extraction TANG Tiancheng
  2026-07-01 14:32 ` [PATCH v3 1/5] disas/riscv: Fix th.srri decoding TANG Tiancheng
@ 2026-07-01 14:32 ` TANG Tiancheng
  2026-07-01 15:01   ` Alex Bennée
  2026-07-01 14:32 ` [PATCH v3 3/5] disas/riscv: Use signed type for vector immediates TANG Tiancheng
                   ` (2 subsequent siblings)
  4 siblings, 1 reply; 9+ messages in thread
From: TANG Tiancheng @ 2026-07-01 14:32 UTC (permalink / raw)
  To: qemu-devel
  Cc: Christoph Muellner, LIU Zhiwei, Palmer Dabbelt, Alistair Francis,
	Daniel Henrique Barboza, Richard Henderson, qemu-riscv,
	TANG Tiancheng

rv_codec_r2_imm6 is used for XThead instructions whose 6-bit
immediate field is encoded in bits 25:20. The old expression
left-shifted by 38 and then right-shifted by 60, so it kept only
bits 25:22.

Shift right by 58 instead, keeping the existing extraction style while
retaining the full bits 25:20 field. This fixes the immediate printed
for th.srri and th.tst.

Fixes: 318df7238b9f ("disas/riscv: Add support for XThead* instructions")
Signed-off-by: TANG Tiancheng <lyndra@linux.alibaba.com>
Reviewed-by: LIU Zhiwei <zhiwei_liu@linux.alibaba.com>
Reviewed-by: Daniel Henrique Barboza <daniel.barboza@oss.qualcomm.com>
---
 disas/riscv.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/disas/riscv.c b/disas/riscv.c
index 7f1b26277378c225c963d8f435cc7a2fc6e4a0b8..400a57083f90be18ccd12415797bce0df974784b 100644
--- a/disas/riscv.c
+++ b/disas/riscv.c
@@ -4546,7 +4546,7 @@ static uint32_t operand_zcmp_rlist(rv_inst inst)
 
 static uint32_t operand_imm6(rv_inst inst)
 {
-    return (inst << 38) >> 60;
+    return (inst << 38) >> 58;
 }
 
 static uint32_t operand_imm2(rv_inst inst)

-- 
2.43.0



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

* Re: [PATCH v3 2/5] disas/riscv: Fix 6-bit immediate extraction
  2026-07-01 14:32 ` [PATCH v3 2/5] disas/riscv: Fix 6-bit immediate extraction TANG Tiancheng
@ 2026-07-01 15:01   ` Alex Bennée
  0 siblings, 0 replies; 9+ messages in thread
From: Alex Bennée @ 2026-07-01 15:01 UTC (permalink / raw)
  To: TANG Tiancheng
  Cc: qemu-devel, Christoph Muellner, LIU Zhiwei, Palmer Dabbelt,
	Alistair Francis, Daniel Henrique Barboza, Richard Henderson,
	qemu-riscv

TANG Tiancheng <lyndra@linux.alibaba.com> writes:

> rv_codec_r2_imm6 is used for XThead instructions whose 6-bit
> immediate field is encoded in bits 25:20. The old expression
> left-shifted by 38 and then right-shifted by 60, so it kept only
> bits 25:22.
>
> Shift right by 58 instead, keeping the existing extraction style while
> retaining the full bits 25:20 field. This fixes the immediate printed
> for th.srri and th.tst.
>
> Fixes: 318df7238b9f ("disas/riscv: Add support for XThead* instructions")
> Signed-off-by: TANG Tiancheng <lyndra@linux.alibaba.com>
> Reviewed-by: LIU Zhiwei <zhiwei_liu@linux.alibaba.com>
> Reviewed-by: Daniel Henrique Barboza <daniel.barboza@oss.qualcomm.com>
> ---
>  disas/riscv.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/disas/riscv.c b/disas/riscv.c
> index 7f1b26277378c225c963d8f435cc7a2fc6e4a0b8..400a57083f90be18ccd12415797bce0df974784b 100644
> --- a/disas/riscv.c
> +++ b/disas/riscv.c
> @@ -4546,7 +4546,7 @@ static uint32_t operand_zcmp_rlist(rv_inst inst)
>  
>  static uint32_t operand_imm6(rv_inst inst)
>  {
> -    return (inst << 38) >> 60;
> +    return (inst << 38) >> 58;

We do have extract32/64 helpers to avoid these manual bit-fiddling.

>  }
>  
>  static uint32_t operand_imm2(rv_inst inst)

-- 
Alex Bennée
Virtualisation Tech Lead @ Linaro


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

* [PATCH v3 3/5] disas/riscv: Use signed type for vector immediates
  2026-07-01 14:32 [PATCH v3 0/5] disas/riscv: Fix immediate decoding and extraction TANG Tiancheng
  2026-07-01 14:32 ` [PATCH v3 1/5] disas/riscv: Fix th.srri decoding TANG Tiancheng
  2026-07-01 14:32 ` [PATCH v3 2/5] disas/riscv: Fix 6-bit immediate extraction TANG Tiancheng
@ 2026-07-01 14:32 ` TANG Tiancheng
  2026-07-01 14:32 ` [PATCH v3 4/5] disas/riscv: Decode unsigned vector immediates as unsigned TANG Tiancheng
  2026-07-01 14:32 ` [PATCH v3 5/5] disas/riscv: Use extract helpers for operand fields TANG Tiancheng
  4 siblings, 0 replies; 9+ messages in thread
From: TANG Tiancheng @ 2026-07-01 14:32 UTC (permalink / raw)
  To: qemu-devel
  Cc: Christoph Muellner, LIU Zhiwei, Palmer Dabbelt, Alistair Francis,
	Daniel Henrique Barboza, Richard Henderson, qemu-riscv,
	TANG Tiancheng

operand_vimm() sign-extends the 5-bit vector immediate field in bits
19:15, but returns uint32_t. This sends negative immediates through an
unsigned type before they are assigned to rv_decode.imm.

Return int32_t to match the signed value extracted by the helper.

Signed-off-by: TANG Tiancheng <lyndra@linux.alibaba.com>
Reviewed-by: LIU Zhiwei <zhiwei_liu@linux.alibaba.com>
---
 disas/riscv.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/disas/riscv.c b/disas/riscv.c
index 400a57083f90be18ccd12415797bce0df974784b..e9dcc173d8fa2f89d85b76595a76931c4dddb462 100644
--- a/disas/riscv.c
+++ b/disas/riscv.c
@@ -4487,7 +4487,7 @@ static uint32_t operand_cimmq(rv_inst inst)
         ((inst << 57) >> 62) << 6;
 }
 
-static uint32_t operand_vimm(rv_inst inst)
+static int32_t operand_vimm(rv_inst inst)
 {
     return (int64_t)(inst << 44) >> 59;
 }

-- 
2.43.0



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

* [PATCH v3 4/5] disas/riscv: Decode unsigned vector immediates as unsigned
  2026-07-01 14:32 [PATCH v3 0/5] disas/riscv: Fix immediate decoding and extraction TANG Tiancheng
                   ` (2 preceding siblings ...)
  2026-07-01 14:32 ` [PATCH v3 3/5] disas/riscv: Use signed type for vector immediates TANG Tiancheng
@ 2026-07-01 14:32 ` TANG Tiancheng
  2026-07-01 14:32 ` [PATCH v3 5/5] disas/riscv: Use extract helpers for operand fields TANG Tiancheng
  4 siblings, 0 replies; 9+ messages in thread
From: TANG Tiancheng @ 2026-07-01 14:32 UTC (permalink / raw)
  To: qemu-devel
  Cc: Christoph Muellner, LIU Zhiwei, Palmer Dabbelt, Alistair Francis,
	Daniel Henrique Barboza, Richard Henderson, qemu-riscv,
	TANG Tiancheng

rv_codec_v_i decodes all .vi operands with operand_vimm(), which
sign-extends bits 19:15. That matches spec operands named imm, but not the
.vi forms whose operand is uimm; uimm=31 is decoded as -1 and printed by
the shared 6-bit 'u' formatter as 63.

Add rv_codec_v_i_u/operand_vuimm() for the 5-bit uimm forms: vsll.vi,
vsrl.vi, vsra.vi, vnsrl.wi, vnsra.wi, vssrl.vi, vssra.vi, vnclipu.wi,
vnclip.wi, vslideup.vi, vslidedown.vi, vrgather.vi, vaeskf1.vi,
vaeskf2.vi, vsm3c.vi, vsm4k.vi and vwsll.vi. The remaining rv_codec_v_i
entries are the signed imm forms.

Fixes: 07f4964d1785 ("disas/riscv.c: rvv: Add disas support for vector instructions")
Fixes: 9d92f56d4a44 ("disas/riscv: Add support for vector crypto extensions")
Signed-off-by: TANG Tiancheng <lyndra@linux.alibaba.com>
Reviewed-by: LIU Zhiwei <zhiwei_liu@linux.alibaba.com>
---
 disas/riscv.c | 45 ++++++++++++++++++++++++++++-----------------
 disas/riscv.h |  1 +
 2 files changed, 29 insertions(+), 17 deletions(-)

diff --git a/disas/riscv.c b/disas/riscv.c
index e9dcc173d8fa2f89d85b76595a76931c4dddb462..4b3f90418f8f17dd3fcb71f70c39607fa293eb0e 100644
--- a/disas/riscv.c
+++ b/disas/riscv.c
@@ -1803,19 +1803,19 @@ const rv_opcode_data rvi_opcode_data[] = {
     { "vxor.vi", rv_codec_v_i, rv_fmt_vd_vs2_imm_vm, NULL, 0, 0, 0 },
     { "vsll.vv", rv_codec_v_r, rv_fmt_vd_vs2_vs1_vm, NULL, 0, 0, 0 },
     { "vsll.vx", rv_codec_v_r, rv_fmt_vd_vs2_rs1_vm, NULL, 0, 0, 0 },
-    { "vsll.vi", rv_codec_v_i, rv_fmt_vd_vs2_uimm_vm, NULL, 0, 0, 0 },
+    { "vsll.vi", rv_codec_v_i_u, rv_fmt_vd_vs2_uimm_vm, NULL, 0, 0, 0 },
     { "vsrl.vv", rv_codec_v_r, rv_fmt_vd_vs2_vs1_vm, NULL, 0, 0, 0 },
     { "vsrl.vx", rv_codec_v_r, rv_fmt_vd_vs2_rs1_vm, NULL, 0, 0, 0 },
-    { "vsrl.vi", rv_codec_v_i, rv_fmt_vd_vs2_uimm_vm, NULL, 0, 0, 0 },
+    { "vsrl.vi", rv_codec_v_i_u, rv_fmt_vd_vs2_uimm_vm, NULL, 0, 0, 0 },
     { "vsra.vv", rv_codec_v_r, rv_fmt_vd_vs2_vs1_vm, NULL, 0, 0, 0 },
     { "vsra.vx", rv_codec_v_r, rv_fmt_vd_vs2_rs1_vm, NULL, 0, 0, 0 },
-    { "vsra.vi", rv_codec_v_i, rv_fmt_vd_vs2_uimm_vm, NULL, 0, 0, 0 },
+    { "vsra.vi", rv_codec_v_i_u, rv_fmt_vd_vs2_uimm_vm, NULL, 0, 0, 0 },
     { "vnsrl.wv", rv_codec_v_r, rv_fmt_vd_vs2_vs1_vm, NULL, 0, 0, 0 },
     { "vnsrl.wx", rv_codec_v_r, rv_fmt_vd_vs2_rs1_vm, NULL, 0, 0, 0 },
-    { "vnsrl.wi", rv_codec_v_i, rv_fmt_vd_vs2_uimm_vm, NULL, 0, 0, 0 },
+    { "vnsrl.wi", rv_codec_v_i_u, rv_fmt_vd_vs2_uimm_vm, NULL, 0, 0, 0 },
     { "vnsra.wv", rv_codec_v_r, rv_fmt_vd_vs2_vs1_vm, NULL, 0, 0, 0 },
     { "vnsra.wx", rv_codec_v_r, rv_fmt_vd_vs2_rs1_vm, NULL, 0, 0, 0 },
-    { "vnsra.wi", rv_codec_v_i, rv_fmt_vd_vs2_uimm_vm, NULL, 0, 0, 0 },
+    { "vnsra.wi", rv_codec_v_i_u, rv_fmt_vd_vs2_uimm_vm, NULL, 0, 0, 0 },
     { "vmseq.vv", rv_codec_v_r, rv_fmt_vd_vs2_vs1_vm, NULL, 0, 0, 0 },
     { "vmseq.vx", rv_codec_v_r, rv_fmt_vd_vs2_rs1_vm, NULL, 0, 0, 0 },
     { "vmseq.vi", rv_codec_v_i, rv_fmt_vd_vs2_imm_vm, NULL, 0, 0, 0 },
@@ -1909,16 +1909,16 @@ const rv_opcode_data rvi_opcode_data[] = {
     { "vsmul.vx", rv_codec_v_r, rv_fmt_vd_vs2_rs1_vm, NULL, 0, 0, 0 },
     { "vssrl.vv", rv_codec_v_r, rv_fmt_vd_vs2_vs1_vm, NULL, 0, 0, 0 },
     { "vssrl.vx", rv_codec_v_r, rv_fmt_vd_vs2_rs1_vm, NULL, 0, 0, 0 },
-    { "vssrl.vi", rv_codec_v_i, rv_fmt_vd_vs2_uimm_vm, NULL, 0, 0, 0 },
+    { "vssrl.vi", rv_codec_v_i_u, rv_fmt_vd_vs2_uimm_vm, NULL, 0, 0, 0 },
     { "vssra.vv", rv_codec_v_r, rv_fmt_vd_vs2_vs1_vm, NULL, 0, 0, 0 },
     { "vssra.vx", rv_codec_v_r, rv_fmt_vd_vs2_rs1_vm, NULL, 0, 0, 0 },
-    { "vssra.vi", rv_codec_v_i, rv_fmt_vd_vs2_uimm_vm, NULL, 0, 0, 0 },
+    { "vssra.vi", rv_codec_v_i_u, rv_fmt_vd_vs2_uimm_vm, NULL, 0, 0, 0 },
     { "vnclipu.wv", rv_codec_v_r, rv_fmt_vd_vs2_vs1_vm, NULL, 0, 0, 0 },
     { "vnclipu.wx", rv_codec_v_r, rv_fmt_vd_vs2_rs1_vm, NULL, 0, 0, 0 },
-    { "vnclipu.wi", rv_codec_v_i, rv_fmt_vd_vs2_uimm_vm, NULL, 0, 0, 0 },
+    { "vnclipu.wi", rv_codec_v_i_u, rv_fmt_vd_vs2_uimm_vm, NULL, 0, 0, 0 },
     { "vnclip.wv", rv_codec_v_r, rv_fmt_vd_vs2_vs1_vm, NULL, 0, 0, 0 },
     { "vnclip.wx", rv_codec_v_r, rv_fmt_vd_vs2_rs1_vm, NULL, 0, 0, 0 },
-    { "vnclip.wi", rv_codec_v_i, rv_fmt_vd_vs2_uimm_vm, NULL, 0, 0, 0 },
+    { "vnclip.wi", rv_codec_v_i_u, rv_fmt_vd_vs2_uimm_vm, NULL, 0, 0, 0 },
     { "vfadd.vv", rv_codec_v_r, rv_fmt_vd_vs2_vs1_vm, NULL, 0, 0, 0 },
     { "vfadd.vf", rv_codec_v_r, rv_fmt_vd_vs2_fs1_vm, NULL, 0, 0, 0 },
     { "vfsub.vv", rv_codec_v_r, rv_fmt_vd_vs2_vs1_vm, NULL, 0, 0, 0 },
@@ -2048,15 +2048,15 @@ const rv_opcode_data rvi_opcode_data[] = {
     { "vfmv.f.s", rv_codec_v_r, rv_fmt_fd_vs2, NULL, 0, 0, 0 },
     { "vfmv.s.f", rv_codec_v_r, rv_fmt_vd_fs1, NULL, 0, 0, 0 },
     { "vslideup.vx", rv_codec_v_r, rv_fmt_vd_vs2_rs1_vm, NULL, 0, 0, 0 },
-    { "vslideup.vi", rv_codec_v_i, rv_fmt_vd_vs2_uimm_vm, NULL, 0, 0, 0 },
+    { "vslideup.vi", rv_codec_v_i_u, rv_fmt_vd_vs2_uimm_vm, NULL, 0, 0, 0 },
     { "vslide1up.vx", rv_codec_v_r, rv_fmt_vd_vs2_rs1_vm, NULL, 0, 0, 0 },
     { "vslidedown.vx", rv_codec_v_r, rv_fmt_vd_vs2_rs1_vm, NULL, 0, 0, 0 },
-    { "vslidedown.vi", rv_codec_v_i, rv_fmt_vd_vs2_uimm_vm, NULL, 0, 0, 0 },
+    { "vslidedown.vi", rv_codec_v_i_u, rv_fmt_vd_vs2_uimm_vm, NULL, 0, 0, 0 },
     { "vslide1down.vx", rv_codec_v_r, rv_fmt_vd_vs2_rs1_vm, NULL, 0, 0, 0 },
     { "vrgather.vv", rv_codec_v_r, rv_fmt_vd_vs2_vs1_vm, NULL, 0, 0, 0 },
     { "vrgatherei16.vv", rv_codec_v_r, rv_fmt_vd_vs2_vs1_vm, NULL, 0, 0, 0 },
     { "vrgather.vx", rv_codec_v_r, rv_fmt_vd_vs2_rs1_vm, NULL, 0, 0, 0 },
-    { "vrgather.vi", rv_codec_v_i, rv_fmt_vd_vs2_uimm_vm, NULL, 0, 0, 0 },
+    { "vrgather.vi", rv_codec_v_i_u, rv_fmt_vd_vs2_uimm_vm, NULL, 0, 0, 0 },
     { "vcompress.vm", rv_codec_v_r, rv_fmt_vd_vs2_vs1, NULL, 0, 0, 0 },
     { "vmv1r.v", rv_codec_v_r, rv_fmt_vd_vs2, NULL, 0, 0, 0 },
     { "vmv2r.v", rv_codec_v_r, rv_fmt_vd_vs2, NULL, 0, 0, 0 },
@@ -2144,8 +2144,8 @@ const rv_opcode_data rvi_opcode_data[] = {
     { "vaesef.vs", rv_codec_v_r, rv_fmt_vd_vs2, NULL, 0, 0, 0 },
     { "vaesem.vv", rv_codec_v_r, rv_fmt_vd_vs2, NULL, 0, 0, 0 },
     { "vaesem.vs", rv_codec_v_r, rv_fmt_vd_vs2, NULL, 0, 0, 0 },
-    { "vaeskf1.vi", rv_codec_v_i, rv_fmt_vd_vs2_uimm, NULL, 0, 0, 0 },
-    { "vaeskf2.vi", rv_codec_v_i, rv_fmt_vd_vs2_uimm, NULL, 0, 0, 0 },
+    { "vaeskf1.vi", rv_codec_v_i_u, rv_fmt_vd_vs2_uimm, NULL, 0, 0, 0 },
+    { "vaeskf2.vi", rv_codec_v_i_u, rv_fmt_vd_vs2_uimm, NULL, 0, 0, 0 },
     { "vaesz.vs", rv_codec_v_r, rv_fmt_vd_vs2, NULL, 0, 0, 0 },
     { "vandn.vv", rv_codec_v_r, rv_fmt_vd_vs2_vs1_vm, NULL, 0, 0, 0 },
     { "vandn.vx", rv_codec_v_r, rv_fmt_vd_vs2_rs1_vm, NULL, 0, 0, 0 },
@@ -2169,14 +2169,14 @@ const rv_opcode_data rvi_opcode_data[] = {
     { "vsha2ch.vv", rv_codec_v_r, rv_fmt_vd_vs2_vs1, NULL, 0, 0, 0 },
     { "vsha2cl.vv", rv_codec_v_r, rv_fmt_vd_vs2_vs1, NULL, 0, 0, 0 },
     { "vsha2ms.vv", rv_codec_v_r, rv_fmt_vd_vs2_vs1, NULL, 0, 0, 0 },
-    { "vsm3c.vi", rv_codec_v_i, rv_fmt_vd_vs2_uimm, NULL, 0, 0, 0 },
+    { "vsm3c.vi", rv_codec_v_i_u, rv_fmt_vd_vs2_uimm, NULL, 0, 0, 0 },
     { "vsm3me.vv", rv_codec_v_r, rv_fmt_vd_vs2_vs1, NULL, 0, 0, 0 },
-    { "vsm4k.vi", rv_codec_v_i, rv_fmt_vd_vs2_uimm, NULL, 0, 0, 0 },
+    { "vsm4k.vi", rv_codec_v_i_u, rv_fmt_vd_vs2_uimm, NULL, 0, 0, 0 },
     { "vsm4r.vv", rv_codec_v_r, rv_fmt_vd_vs2, NULL, 0, 0, 0 },
     { "vsm4r.vs", rv_codec_v_r, rv_fmt_vd_vs2, NULL, 0, 0, 0 },
     { "vwsll.vv", rv_codec_v_r, rv_fmt_vd_vs2_vs1_vm, NULL, 0, 0, 0 },
     { "vwsll.vx", rv_codec_v_r, rv_fmt_vd_vs2_rs1_vm, NULL, 0, 0, 0 },
-    { "vwsll.vi", rv_codec_v_i, rv_fmt_vd_vs2_uimm_vm, NULL, 0, 0, 0 },
+    { "vwsll.vi", rv_codec_v_i_u, rv_fmt_vd_vs2_uimm_vm, NULL, 0, 0, 0 },
     { "amocas.w", rv_codec_r_a, rv_fmt_aqrl_rd_rs2_rs1, NULL, 0, 0, 0 },
     { "amocas.d", rv_codec_r_a, rv_fmt_aqrl_rd_rs2_rs1, NULL, 0, 0, 0 },
     { "amocas.q", rv_codec_r_a, rv_fmt_aqrl_rd_rs2_rs1, NULL, 0, 0, 0 },
@@ -4492,6 +4492,11 @@ static int32_t operand_vimm(rv_inst inst)
     return (int64_t)(inst << 44) >> 59;
 }
 
+static uint32_t operand_vuimm(rv_inst inst)
+{
+    return extract32(inst, 15, 5);
+}
+
 static uint32_t operand_vzimm11(rv_inst inst)
 {
     return (inst << 33) >> 53;
@@ -4895,6 +4900,12 @@ static void decode_inst_operands(rv_decode *dec, rv_isa isa)
         dec->imm = operand_vimm(inst);
         dec->vm = operand_vm(inst);
         break;
+    case rv_codec_v_i_u:
+        dec->rd = operand_rd(inst);
+        dec->rs2 = operand_rs2(inst);
+        dec->imm = operand_vuimm(inst);
+        dec->vm = operand_vm(inst);
+        break;
     case rv_codec_vror_vi:
         dec->rd = operand_rd(inst);
         dec->rs2 = operand_rs2(inst);
diff --git a/disas/riscv.h b/disas/riscv.h
index 379e642ec84e2da12ffd7905b590cdcee7bf215f..91ada8b531eb9a03bae49d8d36fc42c497f277ed 100644
--- a/disas/riscv.h
+++ b/disas/riscv.h
@@ -149,6 +149,7 @@ typedef enum {
     rv_codec_v_r,
     rv_codec_v_ldst,
     rv_codec_v_i,
+    rv_codec_v_i_u,
     rv_codec_vsetvli,
     rv_codec_vsetivli,
     rv_codec_vror_vi,

-- 
2.43.0



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

* [PATCH v3 5/5] disas/riscv: Use extract helpers for operand fields
  2026-07-01 14:32 [PATCH v3 0/5] disas/riscv: Fix immediate decoding and extraction TANG Tiancheng
                   ` (3 preceding siblings ...)
  2026-07-01 14:32 ` [PATCH v3 4/5] disas/riscv: Decode unsigned vector immediates as unsigned TANG Tiancheng
@ 2026-07-01 14:32 ` TANG Tiancheng
  2026-07-01 15:02   ` Alex Bennée
  4 siblings, 1 reply; 9+ messages in thread
From: TANG Tiancheng @ 2026-07-01 14:32 UTC (permalink / raw)
  To: qemu-devel
  Cc: Christoph Muellner, LIU Zhiwei, Palmer Dabbelt, Alistair Francis,
	Daniel Henrique Barboza, Richard Henderson, qemu-riscv,
	TANG Tiancheng

Replace shift-based operand extraction with extract32() and sextract32().
For signed immediates, use sextract32() on the field that carries the sign
bit and combine it with the remaining extract32() fields.

The RISC-V disassembler currently follows target/riscv/internals.h:
insn_len() and decodes only 16-bit or 32-bit instruction lengths, so the
converted fields are all in the low 32 bits of rv_inst.

Suggested-by: Richard Henderson <richard.henderson@linaro.org>
Signed-off-by: TANG Tiancheng <lyndra@linux.alibaba.com>
Reviewed-by: LIU Zhiwei <zhiwei_liu@linux.alibaba.com>
---
 disas/riscv.c | 216 +++++++++++++++++++++++++++++-----------------------------
 1 file changed, 108 insertions(+), 108 deletions(-)

diff --git a/disas/riscv.c b/disas/riscv.c
index 4b3f90418f8f17dd3fcb71f70c39607fa293eb0e..2ba0a6a73ad36c6263eed62119d2c4c864b61cb6 100644
--- a/disas/riscv.c
+++ b/disas/riscv.c
@@ -4202,82 +4202,82 @@ static void decode_inst_opcode(rv_decode *dec, rv_isa isa)
 
 static uint32_t operand_rd(rv_inst inst)
 {
-    return (inst << 52) >> 59;
+    return extract32(inst, 7, 5);
 }
 
 static uint32_t operand_rs1(rv_inst inst)
 {
-    return (inst << 44) >> 59;
+    return extract32(inst, 15, 5);
 }
 
 static uint32_t operand_rs2(rv_inst inst)
 {
-    return (inst << 39) >> 59;
+    return extract32(inst, 20, 5);
 }
 
 static uint32_t operand_rs3(rv_inst inst)
 {
-    return (inst << 32) >> 59;
+    return extract32(inst, 27, 5);
 }
 
 static uint32_t operand_aq(rv_inst inst)
 {
-    return (inst << 37) >> 63;
+    return extract32(inst, 26, 1);
 }
 
 static uint32_t operand_rl(rv_inst inst)
 {
-    return (inst << 38) >> 63;
+    return extract32(inst, 25, 1);
 }
 
 static uint32_t operand_pred(rv_inst inst)
 {
-    return (inst << 36) >> 60;
+    return extract32(inst, 24, 4);
 }
 
 static uint32_t operand_succ(rv_inst inst)
 {
-    return (inst << 40) >> 60;
+    return extract32(inst, 20, 4);
 }
 
 static uint32_t operand_rm(rv_inst inst)
 {
-    return (inst << 49) >> 61;
+    return extract32(inst, 12, 3);
 }
 
 static uint32_t operand_shamt5(rv_inst inst)
 {
-    return (inst << 39) >> 59;
+    return extract32(inst, 20, 5);
 }
 
 static uint32_t operand_shamt6(rv_inst inst)
 {
-    return (inst << 38) >> 58;
+    return extract32(inst, 20, 6);
 }
 
 static uint32_t operand_shamt7(rv_inst inst)
 {
-    return (inst << 37) >> 57;
+    return extract32(inst, 20, 7);
 }
 
 static uint32_t operand_crdq(rv_inst inst)
 {
-    return (inst << 59) >> 61;
+    return extract32(inst, 2, 3);
 }
 
 static uint32_t operand_crs1q(rv_inst inst)
 {
-    return (inst << 54) >> 61;
+    return extract32(inst, 7, 3);
 }
 
 static uint32_t operand_crs1rdq(rv_inst inst)
 {
-    return (inst << 54) >> 61;
+    return extract32(inst, 7, 3);
 }
 
 static uint32_t operand_crs2q(rv_inst inst)
 {
-    return (inst << 59) >> 61;
+    return extract32(inst, 2, 3);
 }
 
 static uint32_t calculate_xreg(uint32_t sreg)
@@ -4287,80 +4287,80 @@ static uint32_t calculate_xreg(uint32_t sreg)
 
 static uint32_t operand_sreg1(rv_inst inst)
 {
-    return calculate_xreg((inst << 54) >> 61);
+    return calculate_xreg(extract32(inst, 7, 3));
 }
 
 static uint32_t operand_sreg2(rv_inst inst)
 {
-    return calculate_xreg((inst << 59) >> 61);
+    return calculate_xreg(extract32(inst, 2, 3));
 }
 
 static uint32_t operand_crd(rv_inst inst)
 {
-    return (inst << 52) >> 59;
+    return extract32(inst, 7, 5);
 }
 
 static uint32_t operand_crs1(rv_inst inst)
 {
-    return (inst << 52) >> 59;
+    return extract32(inst, 7, 5);
 }
 
 static uint32_t operand_crs1rd(rv_inst inst)
 {
-    return (inst << 52) >> 59;
+    return extract32(inst, 7, 5);
 }
 
 static uint32_t operand_crs2(rv_inst inst)
 {
-    return (inst << 57) >> 59;
+    return extract32(inst, 2, 5);
 }
 
 static uint32_t operand_cimmsh5(rv_inst inst)
 {
-    return (inst << 57) >> 59;
+    return extract32(inst, 2, 5);
 }
 
 static uint32_t operand_csr12(rv_inst inst)
 {
-    return (inst << 32) >> 52;
+    return extract32(inst, 20, 12);
 }
 
 static int32_t operand_imm12(rv_inst inst)
 {
-    return ((int64_t)inst << 32) >> 52;
+    return sextract32(inst, 20, 12);
 }
 
 static int32_t operand_imm20(rv_inst inst)
 {
-    return (((int64_t)inst << 32) >> 44) << 12;
+    return sextract32(inst, 12, 20) << 12;
 }
 
 static int32_t operand_jimm20(rv_inst inst)
 {
-    return (((int64_t)inst << 32) >> 63) << 20 |
-        ((inst << 33) >> 54) << 1 |
-        ((inst << 43) >> 63) << 11 |
-        ((inst << 44) >> 56) << 12;
+    return sextract32(inst, 31, 1) << 20 |
+        extract32(inst, 21, 10) << 1 |
+        extract32(inst, 20, 1) << 11 |
+        extract32(inst, 12, 8) << 12;
 }
 
 static int32_t operand_simm12(rv_inst inst)
 {
-    return (((int64_t)inst << 32) >> 57) << 5 |
-        (inst << 52) >> 59;
+    return sextract32(inst, 25, 7) << 5 |
+        extract32(inst, 7, 5);
 }
 
 static int32_t operand_sbimm12(rv_inst inst)
 {
-    return (((int64_t)inst << 32) >> 63) << 12 |
-        ((inst << 33) >> 58) << 5 |
-        ((inst << 52) >> 60) << 1 |
-        ((inst << 56) >> 63) << 11;
+    return sextract32(inst, 31, 1) << 12 |
+        extract32(inst, 25, 6) << 5 |
+        extract32(inst, 8, 4) << 1 |
+        extract32(inst, 7, 1) << 11;
 }
 
 static uint32_t operand_cimmshl6(rv_inst inst, rv_isa isa)
 {
-    int imm = ((inst << 51) >> 63) << 5 |
-        (inst << 57) >> 59;
+    int imm = extract32(inst, 12, 1) << 5 |
+        extract32(inst, 2, 5);
     if (isa == rv128) {
         imm = imm ? imm : 64;
     }
@@ -4369,8 +4369,8 @@ static uint32_t operand_cimmshl6(rv_inst inst, rv_isa isa)
 
 static uint32_t operand_cimmshr6(rv_inst inst, rv_isa isa)
 {
-    int imm = ((inst << 51) >> 63) << 5 |
-        (inst << 57) >> 59;
+    int imm = extract32(inst, 12, 1) << 5 |
+        extract32(inst, 2, 5);
     if (isa == rv128) {
         imm = imm | (imm & 32) << 1;
         imm = imm ? imm : 64;
@@ -4380,116 +4380,116 @@ static uint32_t operand_cimmshr6(rv_inst inst, rv_isa isa)
 
 static int32_t operand_cimmi(rv_inst inst)
 {
-    return (((int64_t)inst << 51) >> 63) << 5 |
-        (inst << 57) >> 59;
+    return sextract32(inst, 12, 1) << 5 |
+        extract32(inst, 2, 5);
 }
 
 static int32_t operand_cimmui(rv_inst inst)
 {
-    return (((int64_t)inst << 51) >> 63) << 17 |
-        ((inst << 57) >> 59) << 12;
+    return sextract32(inst, 12, 1) << 17 |
+        extract32(inst, 2, 5) << 12;
 }
 
 static uint32_t operand_cimmlwsp(rv_inst inst)
 {
-    return ((inst << 51) >> 63) << 5 |
-        ((inst << 57) >> 61) << 2 |
-        ((inst << 60) >> 62) << 6;
+    return extract32(inst, 12, 1) << 5 |
+        extract32(inst, 4, 3) << 2 |
+        extract32(inst, 2, 2) << 6;
 }
 
 static uint32_t operand_cimmldsp(rv_inst inst)
 {
-    return ((inst << 51) >> 63) << 5 |
-        ((inst << 57) >> 62) << 3 |
-        ((inst << 59) >> 61) << 6;
+    return extract32(inst, 12, 1) << 5 |
+        extract32(inst, 5, 2) << 3 |
+        extract32(inst, 2, 3) << 6;
 }
 
 static uint32_t operand_cimmlqsp(rv_inst inst)
 {
-    return ((inst << 51) >> 63) << 5 |
-        ((inst << 57) >> 63) << 4 |
-        ((inst << 58) >> 60) << 6;
+    return extract32(inst, 12, 1) << 5 |
+        extract32(inst, 6, 1) << 4 |
+        extract32(inst, 2, 4) << 6;
 }
 
 static int32_t operand_cimm16sp(rv_inst inst)
 {
-    return (((int64_t)inst << 51) >> 63) << 9 |
-        ((inst << 57) >> 63) << 4 |
-        ((inst << 58) >> 63) << 6 |
-        ((inst << 59) >> 62) << 7 |
-        ((inst << 61) >> 63) << 5;
+    return sextract32(inst, 12, 1) << 9 |
+        extract32(inst, 6, 1) << 4 |
+        extract32(inst, 5, 1) << 6 |
+        extract32(inst, 3, 2) << 7 |
+        extract32(inst, 2, 1) << 5;
 }
 
 static int32_t operand_cimmj(rv_inst inst)
 {
-    return (((int64_t)inst << 51) >> 63) << 11 |
-        ((inst << 52) >> 63) << 4 |
-        ((inst << 53) >> 62) << 8 |
-        ((inst << 55) >> 63) << 10 |
-        ((inst << 56) >> 63) << 6 |
-        ((inst << 57) >> 63) << 7 |
-        ((inst << 58) >> 61) << 1 |
-        ((inst << 61) >> 63) << 5;
+    return sextract32(inst, 12, 1) << 11 |
+        extract32(inst, 11, 1) << 4 |
+        extract32(inst, 9, 2) << 8 |
+        extract32(inst, 8, 1) << 10 |
+        extract32(inst, 7, 1) << 6 |
+        extract32(inst, 6, 1) << 7 |
+        extract32(inst, 3, 3) << 1 |
+        extract32(inst, 2, 1) << 5;
 }
 
 static int32_t operand_cimmb(rv_inst inst)
 {
-    return (((int64_t)inst << 51) >> 63) << 8 |
-        ((inst << 52) >> 62) << 3 |
-        ((inst << 57) >> 62) << 6 |
-        ((inst << 59) >> 62) << 1 |
-        ((inst << 61) >> 63) << 5;
+    return sextract32(inst, 12, 1) << 8 |
+        extract32(inst, 10, 2) << 3 |
+        extract32(inst, 5, 2) << 6 |
+        extract32(inst, 3, 2) << 1 |
+        extract32(inst, 2, 1) << 5;
 }
 
 static uint32_t operand_cimmswsp(rv_inst inst)
 {
-    return ((inst << 51) >> 60) << 2 |
-        ((inst << 55) >> 62) << 6;
+    return extract32(inst, 9, 4) << 2 |
+        extract32(inst, 7, 2) << 6;
 }
 
 static uint32_t operand_cimmsdsp(rv_inst inst)
 {
-    return ((inst << 51) >> 61) << 3 |
-        ((inst << 54) >> 61) << 6;
+    return extract32(inst, 10, 3) << 3 |
+        extract32(inst, 7, 3) << 6;
 }
 
 static uint32_t operand_cimmsqsp(rv_inst inst)
 {
-    return ((inst << 51) >> 62) << 4 |
-        ((inst << 53) >> 60) << 6;
+    return extract32(inst, 11, 2) << 4 |
+        extract32(inst, 7, 4) << 6;
 }
 
 static uint32_t operand_cimm4spn(rv_inst inst)
 {
-    return ((inst << 51) >> 62) << 4 |
-        ((inst << 53) >> 60) << 6 |
-        ((inst << 57) >> 63) << 2 |
-        ((inst << 58) >> 63) << 3;
+    return extract32(inst, 11, 2) << 4 |
+        extract32(inst, 7, 4) << 6 |
+        extract32(inst, 6, 1) << 2 |
+        extract32(inst, 5, 1) << 3;
 }
 
 static uint32_t operand_cimmw(rv_inst inst)
 {
-    return ((inst << 51) >> 61) << 3 |
-        ((inst << 57) >> 63) << 2 |
-        ((inst << 58) >> 63) << 6;
+    return extract32(inst, 10, 3) << 3 |
+        extract32(inst, 6, 1) << 2 |
+        extract32(inst, 5, 1) << 6;
 }
 
 static uint32_t operand_cimmd(rv_inst inst)
 {
-    return ((inst << 51) >> 61) << 3 |
-        ((inst << 57) >> 62) << 6;
+    return extract32(inst, 10, 3) << 3 |
+        extract32(inst, 5, 2) << 6;
 }
 
 static uint32_t operand_cimmq(rv_inst inst)
 {
-    return ((inst << 51) >> 62) << 4 |
-        ((inst << 53) >> 63) << 8 |
-        ((inst << 57) >> 62) << 6;
+    return extract32(inst, 11, 2) << 4 |
+        extract32(inst, 10, 1) << 8 |
+        extract32(inst, 5, 2) << 6;
 }
 
 static int32_t operand_vimm(rv_inst inst)
 {
-    return (int64_t)(inst << 44) >> 59;
+    return sextract32(inst, 15, 5);
 }
 
 static uint32_t operand_vuimm(rv_inst inst)
@@ -4499,74 +4499,74 @@ static uint32_t operand_vuimm(rv_inst inst)
 
 static uint32_t operand_vzimm11(rv_inst inst)
 {
-    return (inst << 33) >> 53;
+    return extract32(inst, 20, 11);
 }
 
 static uint32_t operand_vzimm10(rv_inst inst)
 {
-    return (inst << 34) >> 54;
+    return extract32(inst, 20, 10);
 }
 
 static uint32_t operand_vzimm6(rv_inst inst)
 {
-    return ((inst << 37) >> 63) << 5 |
-        ((inst << 44) >> 59);
+    return extract32(inst, 26, 1) << 5 |
+        extract32(inst, 15, 5);
 }
 
 static uint32_t operand_bs(rv_inst inst)
 {
-    return (inst << 32) >> 62;
+    return extract32(inst, 30, 2);
 }
 
 static uint32_t operand_rnum(rv_inst inst)
 {
-    return (inst << 40) >> 60;
+    return extract32(inst, 20, 4);
 }
 
 static uint32_t operand_vm(rv_inst inst)
 {
-    return (inst << 38) >> 63;
+    return extract32(inst, 25, 1);
 }
 
 static uint32_t operand_uimm_c_lb(rv_inst inst)
 {
-    return (((inst << 58) >> 63) << 1) |
-        ((inst << 57) >> 63);
+    return extract32(inst, 5, 1) << 1 |
+        extract32(inst, 6, 1);
 }
 
 static uint32_t operand_uimm_c_lh(rv_inst inst)
 {
-    return (((inst << 58) >> 63) << 1);
+    return extract32(inst, 5, 1) << 1;
 }
 
 static uint32_t operand_zcmp_spimm(rv_inst inst)
 {
-    return ((inst << 60) >> 62) << 4;
+    return extract32(inst, 2, 2) << 4;
 }
 
 static uint32_t operand_zcmp_rlist(rv_inst inst)
 {
-    return ((inst << 56) >> 60);
+    return extract32(inst, 4, 4);
 }
 
 static uint32_t operand_imm6(rv_inst inst)
 {
-    return (inst << 38) >> 58;
+    return extract32(inst, 20, 6);
 }
 
 static uint32_t operand_imm2(rv_inst inst)
 {
-    return (inst << 37) >> 62;
+    return extract32(inst, 25, 2);
 }
 
 static uint32_t operand_immh(rv_inst inst)
 {
-    return (inst << 32) >> 58;
+    return extract32(inst, 26, 6);
 }
 
 static uint32_t operand_imml(rv_inst inst)
 {
-    return (inst << 38) >> 58;
+    return extract32(inst, 20, 6);
 }
 
 static uint32_t calculate_stack_adj(rv_isa isa, uint32_t rlist, uint32_t spimm)
@@ -4585,12 +4585,12 @@ static uint32_t operand_zcmp_stack_adj(rv_inst inst, rv_isa isa)
 
 static uint32_t operand_tbl_index(rv_inst inst)
 {
-    return ((inst << 54) >> 56);
+    return extract32(inst, 2, 8);
 }
 
 static uint32_t operand_lpl(rv_inst inst)
 {
-    return inst >> 12;
+    return extract32(inst, 12, 20);
 }
 
 /* decode operands */

-- 
2.43.0



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

* Re: [PATCH v3 5/5] disas/riscv: Use extract helpers for operand fields
  2026-07-01 14:32 ` [PATCH v3 5/5] disas/riscv: Use extract helpers for operand fields TANG Tiancheng
@ 2026-07-01 15:02   ` Alex Bennée
  2026-07-03  4:46     ` TianCheng TANG
  0 siblings, 1 reply; 9+ messages in thread
From: Alex Bennée @ 2026-07-01 15:02 UTC (permalink / raw)
  To: TANG Tiancheng
  Cc: qemu-devel, Christoph Muellner, LIU Zhiwei, Palmer Dabbelt,
	Alistair Francis, Daniel Henrique Barboza, Richard Henderson,
	qemu-riscv

TANG Tiancheng <lyndra@linux.alibaba.com> writes:

> Replace shift-based operand extraction with extract32() and sextract32().
> For signed immediates, use sextract32() on the field that carries the sign
> bit and combine it with the remaining extract32() fields.
>
> The RISC-V disassembler currently follows target/riscv/internals.h:
> insn_len() and decodes only 16-bit or 32-bit instruction lengths, so the
> converted fields are all in the low 32 bits of rv_inst.
>
> Suggested-by: Richard Henderson <richard.henderson@linaro.org>
> Signed-off-by: TANG Tiancheng <lyndra@linux.alibaba.com>
> Reviewed-by: LIU Zhiwei <zhiwei_liu@linux.alibaba.com>
> ---
>  disas/riscv.c | 216 +++++++++++++++++++++++++++++-----------------------------
>  1 file changed, 108 insertions(+), 108 deletions(-)
>
> diff --git a/disas/riscv.c b/disas/riscv.c
> index 4b3f90418f8f17dd3fcb71f70c39607fa293eb0e..2ba0a6a73ad36c6263eed62119d2c4c864b61cb6 100644
> --- a/disas/riscv.c
> +++ b/disas/riscv.c
> @@ -4202,82 +4202,82 @@ static void decode_inst_opcode(rv_decode *dec, rv_isa isa)
>  
>  static uint32_t operand_rd(rv_inst inst)
>  {
> -    return (inst << 52) >> 59;
> +    return extract32(inst, 7, 5);
>  }
>  
>  static uint32_t operand_rs1(rv_inst inst)
>  {
> -    return (inst << 44) >> 59;
> +    return extract32(inst, 15, 5);
>  }
>  
>  static uint32_t operand_rs2(rv_inst inst)
>  {
> -    return (inst << 39) >> 59;
> +    return extract32(inst, 20, 5);
>  }
>  
>  static uint32_t operand_rs3(rv_inst inst)
>  {
> -    return (inst << 32) >> 59;
> +    return extract32(inst, 27, 5);
>  }
>  
>  static uint32_t operand_aq(rv_inst inst)
>  {
> -    return (inst << 37) >> 63;
> +    return extract32(inst, 26, 1);
>  }
>  
>  static uint32_t operand_rl(rv_inst inst)
>  {
> -    return (inst << 38) >> 63;
> +    return extract32(inst, 25, 1);
>  }
>  
>  static uint32_t operand_pred(rv_inst inst)
>  {
> -    return (inst << 36) >> 60;
> +    return extract32(inst, 24, 4);
>  }
>  
>  static uint32_t operand_succ(rv_inst inst)
>  {
> -    return (inst << 40) >> 60;
> +    return extract32(inst, 20, 4);
>  }
>  
>  static uint32_t operand_rm(rv_inst inst)
>  {
> -    return (inst << 49) >> 61;
> +    return extract32(inst, 12, 3);
>  }
>  
>  static uint32_t operand_shamt5(rv_inst inst)
>  {
> -    return (inst << 39) >> 59;
> +    return extract32(inst, 20, 5);
>  }
>  
>  static uint32_t operand_shamt6(rv_inst inst)
>  {
> -    return (inst << 38) >> 58;
> +    return extract32(inst, 20, 6);
>  }
>  
>  static uint32_t operand_shamt7(rv_inst inst)
>  {
> -    return (inst << 37) >> 57;
> +    return extract32(inst, 20, 7);
>  }
>  
>  static uint32_t operand_crdq(rv_inst inst)
>  {
> -    return (inst << 59) >> 61;
> +    return extract32(inst, 2, 3);
>  }
>  
>  static uint32_t operand_crs1q(rv_inst inst)
>  {
> -    return (inst << 54) >> 61;
> +    return extract32(inst, 7, 3);
>  }
>  
>  static uint32_t operand_crs1rdq(rv_inst inst)
>  {
> -    return (inst << 54) >> 61;
> +    return extract32(inst, 7, 3);
>  }
>  
>  static uint32_t operand_crs2q(rv_inst inst)
>  {
> -    return (inst << 59) >> 61;
> +    return extract32(inst, 2, 3);
>  }
>  
>  static uint32_t calculate_xreg(uint32_t sreg)
> @@ -4287,80 +4287,80 @@ static uint32_t calculate_xreg(uint32_t sreg)
>  
>  static uint32_t operand_sreg1(rv_inst inst)
>  {
> -    return calculate_xreg((inst << 54) >> 61);
> +    return calculate_xreg(extract32(inst, 7, 3));
>  }
>  
>  static uint32_t operand_sreg2(rv_inst inst)
>  {
> -    return calculate_xreg((inst << 59) >> 61);
> +    return calculate_xreg(extract32(inst, 2, 3));
>  }
>  
>  static uint32_t operand_crd(rv_inst inst)
>  {
> -    return (inst << 52) >> 59;
> +    return extract32(inst, 7, 5);
>  }
>  
>  static uint32_t operand_crs1(rv_inst inst)
>  {
> -    return (inst << 52) >> 59;
> +    return extract32(inst, 7, 5);
>  }
>  
>  static uint32_t operand_crs1rd(rv_inst inst)
>  {
> -    return (inst << 52) >> 59;
> +    return extract32(inst, 7, 5);
>  }
>  
>  static uint32_t operand_crs2(rv_inst inst)
>  {
> -    return (inst << 57) >> 59;
> +    return extract32(inst, 2, 5);
>  }
>  
>  static uint32_t operand_cimmsh5(rv_inst inst)
>  {
> -    return (inst << 57) >> 59;
> +    return extract32(inst, 2, 5);
>  }
>  
>  static uint32_t operand_csr12(rv_inst inst)
>  {
> -    return (inst << 32) >> 52;
> +    return extract32(inst, 20, 12);
>  }
>  
>  static int32_t operand_imm12(rv_inst inst)
>  {
> -    return ((int64_t)inst << 32) >> 52;
> +    return sextract32(inst, 20, 12);
>  }
>  
>  static int32_t operand_imm20(rv_inst inst)
>  {
> -    return (((int64_t)inst << 32) >> 44) << 12;
> +    return sextract32(inst, 12, 20) << 12;
>  }
>  
>  static int32_t operand_jimm20(rv_inst inst)
>  {
> -    return (((int64_t)inst << 32) >> 63) << 20 |
> -        ((inst << 33) >> 54) << 1 |
> -        ((inst << 43) >> 63) << 11 |
> -        ((inst << 44) >> 56) << 12;
> +    return sextract32(inst, 31, 1) << 20 |
> +        extract32(inst, 21, 10) << 1 |
> +        extract32(inst, 20, 1) << 11 |
> +        extract32(inst, 12, 8) << 12;
>  }
>  
>  static int32_t operand_simm12(rv_inst inst)
>  {
> -    return (((int64_t)inst << 32) >> 57) << 5 |
> -        (inst << 52) >> 59;
> +    return sextract32(inst, 25, 7) << 5 |
> +        extract32(inst, 7, 5);
>  }
>  
>  static int32_t operand_sbimm12(rv_inst inst)
>  {
> -    return (((int64_t)inst << 32) >> 63) << 12 |
> -        ((inst << 33) >> 58) << 5 |
> -        ((inst << 52) >> 60) << 1 |
> -        ((inst << 56) >> 63) << 11;
> +    return sextract32(inst, 31, 1) << 12 |
> +        extract32(inst, 25, 6) << 5 |
> +        extract32(inst, 8, 4) << 1 |
> +        extract32(inst, 7, 1) << 11;
>  }
>  
>  static uint32_t operand_cimmshl6(rv_inst inst, rv_isa isa)
>  {
> -    int imm = ((inst << 51) >> 63) << 5 |
> -        (inst << 57) >> 59;
> +    int imm = extract32(inst, 12, 1) << 5 |
> +        extract32(inst, 2, 5);
>      if (isa == rv128) {
>          imm = imm ? imm : 64;
>      }
> @@ -4369,8 +4369,8 @@ static uint32_t operand_cimmshl6(rv_inst inst, rv_isa isa)
>  
>  static uint32_t operand_cimmshr6(rv_inst inst, rv_isa isa)
>  {
> -    int imm = ((inst << 51) >> 63) << 5 |
> -        (inst << 57) >> 59;
> +    int imm = extract32(inst, 12, 1) << 5 |
> +        extract32(inst, 2, 5);
>      if (isa == rv128) {
>          imm = imm | (imm & 32) << 1;
>          imm = imm ? imm : 64;
> @@ -4380,116 +4380,116 @@ static uint32_t operand_cimmshr6(rv_inst inst, rv_isa isa)
>  
>  static int32_t operand_cimmi(rv_inst inst)
>  {
> -    return (((int64_t)inst << 51) >> 63) << 5 |
> -        (inst << 57) >> 59;
> +    return sextract32(inst, 12, 1) << 5 |
> +        extract32(inst, 2, 5);
>  }
>  
>  static int32_t operand_cimmui(rv_inst inst)
>  {
> -    return (((int64_t)inst << 51) >> 63) << 17 |
> -        ((inst << 57) >> 59) << 12;
> +    return sextract32(inst, 12, 1) << 17 |
> +        extract32(inst, 2, 5) << 12;
>  }
>  
>  static uint32_t operand_cimmlwsp(rv_inst inst)
>  {
> -    return ((inst << 51) >> 63) << 5 |
> -        ((inst << 57) >> 61) << 2 |
> -        ((inst << 60) >> 62) << 6;
> +    return extract32(inst, 12, 1) << 5 |
> +        extract32(inst, 4, 3) << 2 |
> +        extract32(inst, 2, 2) << 6;
>  }
>  
>  static uint32_t operand_cimmldsp(rv_inst inst)
>  {
> -    return ((inst << 51) >> 63) << 5 |
> -        ((inst << 57) >> 62) << 3 |
> -        ((inst << 59) >> 61) << 6;
> +    return extract32(inst, 12, 1) << 5 |
> +        extract32(inst, 5, 2) << 3 |
> +        extract32(inst, 2, 3) << 6;
>  }
>  
>  static uint32_t operand_cimmlqsp(rv_inst inst)
>  {
> -    return ((inst << 51) >> 63) << 5 |
> -        ((inst << 57) >> 63) << 4 |
> -        ((inst << 58) >> 60) << 6;
> +    return extract32(inst, 12, 1) << 5 |
> +        extract32(inst, 6, 1) << 4 |
> +        extract32(inst, 2, 4) << 6;
>  }
>  
>  static int32_t operand_cimm16sp(rv_inst inst)
>  {
> -    return (((int64_t)inst << 51) >> 63) << 9 |
> -        ((inst << 57) >> 63) << 4 |
> -        ((inst << 58) >> 63) << 6 |
> -        ((inst << 59) >> 62) << 7 |
> -        ((inst << 61) >> 63) << 5;
> +    return sextract32(inst, 12, 1) << 9 |
> +        extract32(inst, 6, 1) << 4 |
> +        extract32(inst, 5, 1) << 6 |
> +        extract32(inst, 3, 2) << 7 |
> +        extract32(inst, 2, 1) << 5;
>  }
>  
>  static int32_t operand_cimmj(rv_inst inst)
>  {
> -    return (((int64_t)inst << 51) >> 63) << 11 |
> -        ((inst << 52) >> 63) << 4 |
> -        ((inst << 53) >> 62) << 8 |
> -        ((inst << 55) >> 63) << 10 |
> -        ((inst << 56) >> 63) << 6 |
> -        ((inst << 57) >> 63) << 7 |
> -        ((inst << 58) >> 61) << 1 |
> -        ((inst << 61) >> 63) << 5;
> +    return sextract32(inst, 12, 1) << 11 |
> +        extract32(inst, 11, 1) << 4 |
> +        extract32(inst, 9, 2) << 8 |
> +        extract32(inst, 8, 1) << 10 |
> +        extract32(inst, 7, 1) << 6 |
> +        extract32(inst, 6, 1) << 7 |
> +        extract32(inst, 3, 3) << 1 |
> +        extract32(inst, 2, 1) << 5;
>  }
>  
>  static int32_t operand_cimmb(rv_inst inst)
>  {
> -    return (((int64_t)inst << 51) >> 63) << 8 |
> -        ((inst << 52) >> 62) << 3 |
> -        ((inst << 57) >> 62) << 6 |
> -        ((inst << 59) >> 62) << 1 |
> -        ((inst << 61) >> 63) << 5;
> +    return sextract32(inst, 12, 1) << 8 |
> +        extract32(inst, 10, 2) << 3 |
> +        extract32(inst, 5, 2) << 6 |
> +        extract32(inst, 3, 2) << 1 |
> +        extract32(inst, 2, 1) << 5;
>  }
>  
>  static uint32_t operand_cimmswsp(rv_inst inst)
>  {
> -    return ((inst << 51) >> 60) << 2 |
> -        ((inst << 55) >> 62) << 6;
> +    return extract32(inst, 9, 4) << 2 |
> +        extract32(inst, 7, 2) << 6;
>  }
>  
>  static uint32_t operand_cimmsdsp(rv_inst inst)
>  {
> -    return ((inst << 51) >> 61) << 3 |
> -        ((inst << 54) >> 61) << 6;
> +    return extract32(inst, 10, 3) << 3 |
> +        extract32(inst, 7, 3) << 6;
>  }
>  
>  static uint32_t operand_cimmsqsp(rv_inst inst)
>  {
> -    return ((inst << 51) >> 62) << 4 |
> -        ((inst << 53) >> 60) << 6;
> +    return extract32(inst, 11, 2) << 4 |
> +        extract32(inst, 7, 4) << 6;
>  }
>  
>  static uint32_t operand_cimm4spn(rv_inst inst)
>  {
> -    return ((inst << 51) >> 62) << 4 |
> -        ((inst << 53) >> 60) << 6 |
> -        ((inst << 57) >> 63) << 2 |
> -        ((inst << 58) >> 63) << 3;
> +    return extract32(inst, 11, 2) << 4 |
> +        extract32(inst, 7, 4) << 6 |
> +        extract32(inst, 6, 1) << 2 |
> +        extract32(inst, 5, 1) << 3;
>  }
>  
>  static uint32_t operand_cimmw(rv_inst inst)
>  {
> -    return ((inst << 51) >> 61) << 3 |
> -        ((inst << 57) >> 63) << 2 |
> -        ((inst << 58) >> 63) << 6;
> +    return extract32(inst, 10, 3) << 3 |
> +        extract32(inst, 6, 1) << 2 |
> +        extract32(inst, 5, 1) << 6;
>  }
>  
>  static uint32_t operand_cimmd(rv_inst inst)
>  {
> -    return ((inst << 51) >> 61) << 3 |
> -        ((inst << 57) >> 62) << 6;
> +    return extract32(inst, 10, 3) << 3 |
> +        extract32(inst, 5, 2) << 6;
>  }
>  
>  static uint32_t operand_cimmq(rv_inst inst)
>  {
> -    return ((inst << 51) >> 62) << 4 |
> -        ((inst << 53) >> 63) << 8 |
> -        ((inst << 57) >> 62) << 6;
> +    return extract32(inst, 11, 2) << 4 |
> +        extract32(inst, 10, 1) << 8 |
> +        extract32(inst, 5, 2) << 6;
>  }
>  
>  static int32_t operand_vimm(rv_inst inst)
>  {
> -    return (int64_t)(inst << 44) >> 59;
> +    return sextract32(inst, 15, 5);
>  }
>  
>  static uint32_t operand_vuimm(rv_inst inst)
> @@ -4499,74 +4499,74 @@ static uint32_t operand_vuimm(rv_inst inst)
>  
>  static uint32_t operand_vzimm11(rv_inst inst)
>  {
> -    return (inst << 33) >> 53;
> +    return extract32(inst, 20, 11);
>  }
>  
>  static uint32_t operand_vzimm10(rv_inst inst)
>  {
> -    return (inst << 34) >> 54;
> +    return extract32(inst, 20, 10);
>  }
>  
>  static uint32_t operand_vzimm6(rv_inst inst)
>  {
> -    return ((inst << 37) >> 63) << 5 |
> -        ((inst << 44) >> 59);
> +    return extract32(inst, 26, 1) << 5 |
> +        extract32(inst, 15, 5);
>  }
>  
>  static uint32_t operand_bs(rv_inst inst)
>  {
> -    return (inst << 32) >> 62;
> +    return extract32(inst, 30, 2);
>  }
>  
>  static uint32_t operand_rnum(rv_inst inst)
>  {
> -    return (inst << 40) >> 60;
> +    return extract32(inst, 20, 4);
>  }
>  
>  static uint32_t operand_vm(rv_inst inst)
>  {
> -    return (inst << 38) >> 63;
> +    return extract32(inst, 25, 1);
>  }
>  
>  static uint32_t operand_uimm_c_lb(rv_inst inst)
>  {
> -    return (((inst << 58) >> 63) << 1) |
> -        ((inst << 57) >> 63);
> +    return extract32(inst, 5, 1) << 1 |
> +        extract32(inst, 6, 1);
>  }
>  
>  static uint32_t operand_uimm_c_lh(rv_inst inst)
>  {
> -    return (((inst << 58) >> 63) << 1);
> +    return extract32(inst, 5, 1) << 1;
>  }
>  
>  static uint32_t operand_zcmp_spimm(rv_inst inst)
>  {
> -    return ((inst << 60) >> 62) << 4;
> +    return extract32(inst, 2, 2) << 4;
>  }
>  
>  static uint32_t operand_zcmp_rlist(rv_inst inst)
>  {
> -    return ((inst << 56) >> 60);
> +    return extract32(inst, 4, 4);
>  }
>  
>  static uint32_t operand_imm6(rv_inst inst)
>  {
> -    return (inst << 38) >> 58;
> +    return extract32(inst, 20, 6);
>  }

Ahh there it is, I would have just fixed it directly.

>  
>  static uint32_t operand_imm2(rv_inst inst)
>  {
> -    return (inst << 37) >> 62;
> +    return extract32(inst, 25, 2);
>  }
>  
>  static uint32_t operand_immh(rv_inst inst)
>  {
> -    return (inst << 32) >> 58;
> +    return extract32(inst, 26, 6);
>  }
>  
>  static uint32_t operand_imml(rv_inst inst)
>  {
> -    return (inst << 38) >> 58;
> +    return extract32(inst, 20, 6);
>  }
>  
>  static uint32_t calculate_stack_adj(rv_isa isa, uint32_t rlist, uint32_t spimm)
> @@ -4585,12 +4585,12 @@ static uint32_t operand_zcmp_stack_adj(rv_inst inst, rv_isa isa)
>  
>  static uint32_t operand_tbl_index(rv_inst inst)
>  {
> -    return ((inst << 54) >> 56);
> +    return extract32(inst, 2, 8);
>  }
>  
>  static uint32_t operand_lpl(rv_inst inst)
>  {
> -    return inst >> 12;
> +    return extract32(inst, 12, 20);
>  }
>  
>  /* decode operands */

-- 
Alex Bennée
Virtualisation Tech Lead @ Linaro


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

* Re: [PATCH v3 5/5] disas/riscv: Use extract helpers for operand fields
  2026-07-01 15:02   ` Alex Bennée
@ 2026-07-03  4:46     ` TianCheng TANG
  0 siblings, 0 replies; 9+ messages in thread
From: TianCheng TANG @ 2026-07-03  4:46 UTC (permalink / raw)
  To: Alex Bennée
  Cc: qemu-devel, Christoph Muellner, LIU Zhiwei, Palmer Dabbelt,
	Alistair Francis, Daniel Henrique Barboza, Richard Henderson,
	qemu-riscv


在 2026/7/1 23:02, Alex Bennée 写道:
> TANG Tiancheng <lyndra@linux.alibaba.com> writes:
>
>> Replace shift-based operand extraction with extract32() and sextract32().
>> For signed immediates, use sextract32() on the field that carries the sign
>> bit and combine it with the remaining extract32() fields.
>>
>> The RISC-V disassembler currently follows target/riscv/internals.h:
>> insn_len() and decodes only 16-bit or 32-bit instruction lengths, so the
>> converted fields are all in the low 32 bits of rv_inst.
>>
>> Suggested-by: Richard Henderson <richard.henderson@linaro.org>
>> Signed-off-by: TANG Tiancheng <lyndra@linux.alibaba.com>
>> Reviewed-by: LIU Zhiwei <zhiwei_liu@linux.alibaba.com>
>> ---
>>   disas/riscv.c | 216 +++++++++++++++++++++++++++++-----------------------------
>>   1 file changed, 108 insertions(+), 108 deletions(-)
>>
>> diff --git a/disas/riscv.c b/disas/riscv.c
>> index 4b3f90418f8f17dd3fcb71f70c39607fa293eb0e..2ba0a6a73ad36c6263eed62119d2c4c864b61cb6 100644
>> --- a/disas/riscv.c
>> +++ b/disas/riscv.c
>> @@ -4202,82 +4202,82 @@ static void decode_inst_opcode(rv_decode *dec, rv_isa isa)
>>   
>>   static uint32_t operand_rd(rv_inst inst)
>>   {
>> -    return (inst << 52) >> 59;
>> +    return extract32(inst, 7, 5);
>>   }
>>   
>>   static uint32_t operand_rs1(rv_inst inst)
>>   {
>> -    return (inst << 44) >> 59;
>> +    return extract32(inst, 15, 5);
>>   }
>>   
>>   static uint32_t operand_rs2(rv_inst inst)
>>   {
>> -    return (inst << 39) >> 59;
>> +    return extract32(inst, 20, 5);
>>   }
>>   
>>   static uint32_t operand_rs3(rv_inst inst)
>>   {
>> -    return (inst << 32) >> 59;
>> +    return extract32(inst, 27, 5);
>>   }
>>   
>>   static uint32_t operand_aq(rv_inst inst)
>>   {
>> -    return (inst << 37) >> 63;
>> +    return extract32(inst, 26, 1);
>>   }
>>   
>>   static uint32_t operand_rl(rv_inst inst)
>>   {
>> -    return (inst << 38) >> 63;
>> +    return extract32(inst, 25, 1);
>>   }
>>   
>>   static uint32_t operand_pred(rv_inst inst)
>>   {
>> -    return (inst << 36) >> 60;
>> +    return extract32(inst, 24, 4);
>>   }
>>   
>>   static uint32_t operand_succ(rv_inst inst)
>>   {
>> -    return (inst << 40) >> 60;
>> +    return extract32(inst, 20, 4);
>>   }
>>   
>>   static uint32_t operand_rm(rv_inst inst)
>>   {
>> -    return (inst << 49) >> 61;
>> +    return extract32(inst, 12, 3);
>>   }
>>   
>>   static uint32_t operand_shamt5(rv_inst inst)
>>   {
>> -    return (inst << 39) >> 59;
>> +    return extract32(inst, 20, 5);
>>   }
>>   
>>   static uint32_t operand_shamt6(rv_inst inst)
>>   {
>> -    return (inst << 38) >> 58;
>> +    return extract32(inst, 20, 6);
>>   }
>>   
>>   static uint32_t operand_shamt7(rv_inst inst)
>>   {
>> -    return (inst << 37) >> 57;
>> +    return extract32(inst, 20, 7);
>>   }
>>   
>>   static uint32_t operand_crdq(rv_inst inst)
>>   {
>> -    return (inst << 59) >> 61;
>> +    return extract32(inst, 2, 3);
>>   }
>>   
>>   static uint32_t operand_crs1q(rv_inst inst)
>>   {
>> -    return (inst << 54) >> 61;
>> +    return extract32(inst, 7, 3);
>>   }
>>   
>>   static uint32_t operand_crs1rdq(rv_inst inst)
>>   {
>> -    return (inst << 54) >> 61;
>> +    return extract32(inst, 7, 3);
>>   }
>>   
>>   static uint32_t operand_crs2q(rv_inst inst)
>>   {
>> -    return (inst << 59) >> 61;
>> +    return extract32(inst, 2, 3);
>>   }
>>   
>>   static uint32_t calculate_xreg(uint32_t sreg)
>> @@ -4287,80 +4287,80 @@ static uint32_t calculate_xreg(uint32_t sreg)
>>   
>>   static uint32_t operand_sreg1(rv_inst inst)
>>   {
>> -    return calculate_xreg((inst << 54) >> 61);
>> +    return calculate_xreg(extract32(inst, 7, 3));
>>   }
>>   
>>   static uint32_t operand_sreg2(rv_inst inst)
>>   {
>> -    return calculate_xreg((inst << 59) >> 61);
>> +    return calculate_xreg(extract32(inst, 2, 3));
>>   }
>>   
>>   static uint32_t operand_crd(rv_inst inst)
>>   {
>> -    return (inst << 52) >> 59;
>> +    return extract32(inst, 7, 5);
>>   }
>>   
>>   static uint32_t operand_crs1(rv_inst inst)
>>   {
>> -    return (inst << 52) >> 59;
>> +    return extract32(inst, 7, 5);
>>   }
>>   
>>   static uint32_t operand_crs1rd(rv_inst inst)
>>   {
>> -    return (inst << 52) >> 59;
>> +    return extract32(inst, 7, 5);
>>   }
>>   
>>   static uint32_t operand_crs2(rv_inst inst)
>>   {
>> -    return (inst << 57) >> 59;
>> +    return extract32(inst, 2, 5);
>>   }
>>   
>>   static uint32_t operand_cimmsh5(rv_inst inst)
>>   {
>> -    return (inst << 57) >> 59;
>> +    return extract32(inst, 2, 5);
>>   }
>>   
>>   static uint32_t operand_csr12(rv_inst inst)
>>   {
>> -    return (inst << 32) >> 52;
>> +    return extract32(inst, 20, 12);
>>   }
>>   
>>   static int32_t operand_imm12(rv_inst inst)
>>   {
>> -    return ((int64_t)inst << 32) >> 52;
>> +    return sextract32(inst, 20, 12);
>>   }
>>   
>>   static int32_t operand_imm20(rv_inst inst)
>>   {
>> -    return (((int64_t)inst << 32) >> 44) << 12;
>> +    return sextract32(inst, 12, 20) << 12;
>>   }
>>   
>>   static int32_t operand_jimm20(rv_inst inst)
>>   {
>> -    return (((int64_t)inst << 32) >> 63) << 20 |
>> -        ((inst << 33) >> 54) << 1 |
>> -        ((inst << 43) >> 63) << 11 |
>> -        ((inst << 44) >> 56) << 12;
>> +    return sextract32(inst, 31, 1) << 20 |
>> +        extract32(inst, 21, 10) << 1 |
>> +        extract32(inst, 20, 1) << 11 |
>> +        extract32(inst, 12, 8) << 12;
>>   }
>>   
>>   static int32_t operand_simm12(rv_inst inst)
>>   {
>> -    return (((int64_t)inst << 32) >> 57) << 5 |
>> -        (inst << 52) >> 59;
>> +    return sextract32(inst, 25, 7) << 5 |
>> +        extract32(inst, 7, 5);
>>   }
>>   
>>   static int32_t operand_sbimm12(rv_inst inst)
>>   {
>> -    return (((int64_t)inst << 32) >> 63) << 12 |
>> -        ((inst << 33) >> 58) << 5 |
>> -        ((inst << 52) >> 60) << 1 |
>> -        ((inst << 56) >> 63) << 11;
>> +    return sextract32(inst, 31, 1) << 12 |
>> +        extract32(inst, 25, 6) << 5 |
>> +        extract32(inst, 8, 4) << 1 |
>> +        extract32(inst, 7, 1) << 11;
>>   }
>>   
>>   static uint32_t operand_cimmshl6(rv_inst inst, rv_isa isa)
>>   {
>> -    int imm = ((inst << 51) >> 63) << 5 |
>> -        (inst << 57) >> 59;
>> +    int imm = extract32(inst, 12, 1) << 5 |
>> +        extract32(inst, 2, 5);
>>       if (isa == rv128) {
>>           imm = imm ? imm : 64;
>>       }
>> @@ -4369,8 +4369,8 @@ static uint32_t operand_cimmshl6(rv_inst inst, rv_isa isa)
>>   
>>   static uint32_t operand_cimmshr6(rv_inst inst, rv_isa isa)
>>   {
>> -    int imm = ((inst << 51) >> 63) << 5 |
>> -        (inst << 57) >> 59;
>> +    int imm = extract32(inst, 12, 1) << 5 |
>> +        extract32(inst, 2, 5);
>>       if (isa == rv128) {
>>           imm = imm | (imm & 32) << 1;
>>           imm = imm ? imm : 64;
>> @@ -4380,116 +4380,116 @@ static uint32_t operand_cimmshr6(rv_inst inst, rv_isa isa)
>>   
>>   static int32_t operand_cimmi(rv_inst inst)
>>   {
>> -    return (((int64_t)inst << 51) >> 63) << 5 |
>> -        (inst << 57) >> 59;
>> +    return sextract32(inst, 12, 1) << 5 |
>> +        extract32(inst, 2, 5);
>>   }
>>   
>>   static int32_t operand_cimmui(rv_inst inst)
>>   {
>> -    return (((int64_t)inst << 51) >> 63) << 17 |
>> -        ((inst << 57) >> 59) << 12;
>> +    return sextract32(inst, 12, 1) << 17 |
>> +        extract32(inst, 2, 5) << 12;
>>   }
>>   
>>   static uint32_t operand_cimmlwsp(rv_inst inst)
>>   {
>> -    return ((inst << 51) >> 63) << 5 |
>> -        ((inst << 57) >> 61) << 2 |
>> -        ((inst << 60) >> 62) << 6;
>> +    return extract32(inst, 12, 1) << 5 |
>> +        extract32(inst, 4, 3) << 2 |
>> +        extract32(inst, 2, 2) << 6;
>>   }
>>   
>>   static uint32_t operand_cimmldsp(rv_inst inst)
>>   {
>> -    return ((inst << 51) >> 63) << 5 |
>> -        ((inst << 57) >> 62) << 3 |
>> -        ((inst << 59) >> 61) << 6;
>> +    return extract32(inst, 12, 1) << 5 |
>> +        extract32(inst, 5, 2) << 3 |
>> +        extract32(inst, 2, 3) << 6;
>>   }
>>   
>>   static uint32_t operand_cimmlqsp(rv_inst inst)
>>   {
>> -    return ((inst << 51) >> 63) << 5 |
>> -        ((inst << 57) >> 63) << 4 |
>> -        ((inst << 58) >> 60) << 6;
>> +    return extract32(inst, 12, 1) << 5 |
>> +        extract32(inst, 6, 1) << 4 |
>> +        extract32(inst, 2, 4) << 6;
>>   }
>>   
>>   static int32_t operand_cimm16sp(rv_inst inst)
>>   {
>> -    return (((int64_t)inst << 51) >> 63) << 9 |
>> -        ((inst << 57) >> 63) << 4 |
>> -        ((inst << 58) >> 63) << 6 |
>> -        ((inst << 59) >> 62) << 7 |
>> -        ((inst << 61) >> 63) << 5;
>> +    return sextract32(inst, 12, 1) << 9 |
>> +        extract32(inst, 6, 1) << 4 |
>> +        extract32(inst, 5, 1) << 6 |
>> +        extract32(inst, 3, 2) << 7 |
>> +        extract32(inst, 2, 1) << 5;
>>   }
>>   
>>   static int32_t operand_cimmj(rv_inst inst)
>>   {
>> -    return (((int64_t)inst << 51) >> 63) << 11 |
>> -        ((inst << 52) >> 63) << 4 |
>> -        ((inst << 53) >> 62) << 8 |
>> -        ((inst << 55) >> 63) << 10 |
>> -        ((inst << 56) >> 63) << 6 |
>> -        ((inst << 57) >> 63) << 7 |
>> -        ((inst << 58) >> 61) << 1 |
>> -        ((inst << 61) >> 63) << 5;
>> +    return sextract32(inst, 12, 1) << 11 |
>> +        extract32(inst, 11, 1) << 4 |
>> +        extract32(inst, 9, 2) << 8 |
>> +        extract32(inst, 8, 1) << 10 |
>> +        extract32(inst, 7, 1) << 6 |
>> +        extract32(inst, 6, 1) << 7 |
>> +        extract32(inst, 3, 3) << 1 |
>> +        extract32(inst, 2, 1) << 5;
>>   }
>>   
>>   static int32_t operand_cimmb(rv_inst inst)
>>   {
>> -    return (((int64_t)inst << 51) >> 63) << 8 |
>> -        ((inst << 52) >> 62) << 3 |
>> -        ((inst << 57) >> 62) << 6 |
>> -        ((inst << 59) >> 62) << 1 |
>> -        ((inst << 61) >> 63) << 5;
>> +    return sextract32(inst, 12, 1) << 8 |
>> +        extract32(inst, 10, 2) << 3 |
>> +        extract32(inst, 5, 2) << 6 |
>> +        extract32(inst, 3, 2) << 1 |
>> +        extract32(inst, 2, 1) << 5;
>>   }
>>   
>>   static uint32_t operand_cimmswsp(rv_inst inst)
>>   {
>> -    return ((inst << 51) >> 60) << 2 |
>> -        ((inst << 55) >> 62) << 6;
>> +    return extract32(inst, 9, 4) << 2 |
>> +        extract32(inst, 7, 2) << 6;
>>   }
>>   
>>   static uint32_t operand_cimmsdsp(rv_inst inst)
>>   {
>> -    return ((inst << 51) >> 61) << 3 |
>> -        ((inst << 54) >> 61) << 6;
>> +    return extract32(inst, 10, 3) << 3 |
>> +        extract32(inst, 7, 3) << 6;
>>   }
>>   
>>   static uint32_t operand_cimmsqsp(rv_inst inst)
>>   {
>> -    return ((inst << 51) >> 62) << 4 |
>> -        ((inst << 53) >> 60) << 6;
>> +    return extract32(inst, 11, 2) << 4 |
>> +        extract32(inst, 7, 4) << 6;
>>   }
>>   
>>   static uint32_t operand_cimm4spn(rv_inst inst)
>>   {
>> -    return ((inst << 51) >> 62) << 4 |
>> -        ((inst << 53) >> 60) << 6 |
>> -        ((inst << 57) >> 63) << 2 |
>> -        ((inst << 58) >> 63) << 3;
>> +    return extract32(inst, 11, 2) << 4 |
>> +        extract32(inst, 7, 4) << 6 |
>> +        extract32(inst, 6, 1) << 2 |
>> +        extract32(inst, 5, 1) << 3;
>>   }
>>   
>>   static uint32_t operand_cimmw(rv_inst inst)
>>   {
>> -    return ((inst << 51) >> 61) << 3 |
>> -        ((inst << 57) >> 63) << 2 |
>> -        ((inst << 58) >> 63) << 6;
>> +    return extract32(inst, 10, 3) << 3 |
>> +        extract32(inst, 6, 1) << 2 |
>> +        extract32(inst, 5, 1) << 6;
>>   }
>>   
>>   static uint32_t operand_cimmd(rv_inst inst)
>>   {
>> -    return ((inst << 51) >> 61) << 3 |
>> -        ((inst << 57) >> 62) << 6;
>> +    return extract32(inst, 10, 3) << 3 |
>> +        extract32(inst, 5, 2) << 6;
>>   }
>>   
>>   static uint32_t operand_cimmq(rv_inst inst)
>>   {
>> -    return ((inst << 51) >> 62) << 4 |
>> -        ((inst << 53) >> 63) << 8 |
>> -        ((inst << 57) >> 62) << 6;
>> +    return extract32(inst, 11, 2) << 4 |
>> +        extract32(inst, 10, 1) << 8 |
>> +        extract32(inst, 5, 2) << 6;
>>   }
>>   
>>   static int32_t operand_vimm(rv_inst inst)
>>   {
>> -    return (int64_t)(inst << 44) >> 59;
>> +    return sextract32(inst, 15, 5);
>>   }
>>   
>>   static uint32_t operand_vuimm(rv_inst inst)
>> @@ -4499,74 +4499,74 @@ static uint32_t operand_vuimm(rv_inst inst)
>>   
>>   static uint32_t operand_vzimm11(rv_inst inst)
>>   {
>> -    return (inst << 33) >> 53;
>> +    return extract32(inst, 20, 11);
>>   }
>>   
>>   static uint32_t operand_vzimm10(rv_inst inst)
>>   {
>> -    return (inst << 34) >> 54;
>> +    return extract32(inst, 20, 10);
>>   }
>>   
>>   static uint32_t operand_vzimm6(rv_inst inst)
>>   {
>> -    return ((inst << 37) >> 63) << 5 |
>> -        ((inst << 44) >> 59);
>> +    return extract32(inst, 26, 1) << 5 |
>> +        extract32(inst, 15, 5);
>>   }
>>   
>>   static uint32_t operand_bs(rv_inst inst)
>>   {
>> -    return (inst << 32) >> 62;
>> +    return extract32(inst, 30, 2);
>>   }
>>   
>>   static uint32_t operand_rnum(rv_inst inst)
>>   {
>> -    return (inst << 40) >> 60;
>> +    return extract32(inst, 20, 4);
>>   }
>>   
>>   static uint32_t operand_vm(rv_inst inst)
>>   {
>> -    return (inst << 38) >> 63;
>> +    return extract32(inst, 25, 1);
>>   }
>>   
>>   static uint32_t operand_uimm_c_lb(rv_inst inst)
>>   {
>> -    return (((inst << 58) >> 63) << 1) |
>> -        ((inst << 57) >> 63);
>> +    return extract32(inst, 5, 1) << 1 |
>> +        extract32(inst, 6, 1);
>>   }
>>   
>>   static uint32_t operand_uimm_c_lh(rv_inst inst)
>>   {
>> -    return (((inst << 58) >> 63) << 1);
>> +    return extract32(inst, 5, 1) << 1;
>>   }
>>   
>>   static uint32_t operand_zcmp_spimm(rv_inst inst)
>>   {
>> -    return ((inst << 60) >> 62) << 4;
>> +    return extract32(inst, 2, 2) << 4;
>>   }
>>   
>>   static uint32_t operand_zcmp_rlist(rv_inst inst)
>>   {
>> -    return ((inst << 56) >> 60);
>> +    return extract32(inst, 4, 4);
>>   }
>>   
>>   static uint32_t operand_imm6(rv_inst inst)
>>   {
>> -    return (inst << 38) >> 58;
>> +    return extract32(inst, 20, 6);
>>   }
> Ahh there it is, I would have just fixed it directly.
Good point, I'll do that in v4.

I'll make the fix patch use extract32(inst, 20, 6) directly, instead of
adding the shift-based fix first and converting it in the cleanup patch.

I'll add your Suggested-by tag there too.

Thanks,
TianCheng
>
>>   
>>   static uint32_t operand_imm2(rv_inst inst)
>>   {
>> -    return (inst << 37) >> 62;
>> +    return extract32(inst, 25, 2);
>>   }
>>   
>>   static uint32_t operand_immh(rv_inst inst)
>>   {
>> -    return (inst << 32) >> 58;
>> +    return extract32(inst, 26, 6);
>>   }
>>   
>>   static uint32_t operand_imml(rv_inst inst)
>>   {
>> -    return (inst << 38) >> 58;
>> +    return extract32(inst, 20, 6);
>>   }
>>   
>>   static uint32_t calculate_stack_adj(rv_isa isa, uint32_t rlist, uint32_t spimm)
>> @@ -4585,12 +4585,12 @@ static uint32_t operand_zcmp_stack_adj(rv_inst inst, rv_isa isa)
>>   
>>   static uint32_t operand_tbl_index(rv_inst inst)
>>   {
>> -    return ((inst << 54) >> 56);
>> +    return extract32(inst, 2, 8);
>>   }
>>   
>>   static uint32_t operand_lpl(rv_inst inst)
>>   {
>> -    return inst >> 12;
>> +    return extract32(inst, 12, 20);
>>   }
>>   
>>   /* decode operands */


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

end of thread, other threads:[~2026-07-03  4:46 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-07-01 14:32 [PATCH v3 0/5] disas/riscv: Fix immediate decoding and extraction TANG Tiancheng
2026-07-01 14:32 ` [PATCH v3 1/5] disas/riscv: Fix th.srri decoding TANG Tiancheng
2026-07-01 14:32 ` [PATCH v3 2/5] disas/riscv: Fix 6-bit immediate extraction TANG Tiancheng
2026-07-01 15:01   ` Alex Bennée
2026-07-01 14:32 ` [PATCH v3 3/5] disas/riscv: Use signed type for vector immediates TANG Tiancheng
2026-07-01 14:32 ` [PATCH v3 4/5] disas/riscv: Decode unsigned vector immediates as unsigned TANG Tiancheng
2026-07-01 14:32 ` [PATCH v3 5/5] disas/riscv: Use extract helpers for operand fields TANG Tiancheng
2026-07-01 15:02   ` Alex Bennée
2026-07-03  4:46     ` TianCheng TANG

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.