Re: [PATCH 2/3] disas/riscv.c: Correct disasm of lq

Daniel Henrique Barboza <[email protected]>
Newsgroups org.nongnu.qemu-riscv,org.nongnu.qemu-devel
Message-ID <[email protected]>

On 7/15/2026 4:07 PM, [email protected] wrote:
> From: Frédéric Pétrot <[email protected]>
> 
> Lq shares the cmo opcode space, but the cbos must have rd = 0.
> Now ensure that they are matched only in that case.
> This implies that lq can be recognized as such only when rd != 0.
> Update the decoder file accordingly.
> 

Fixes: 9273cda722 ("disas/riscv.c: add 'cbo' insns to disassembler")

> Reported-by: Julien Thillard <[email protected]>
> Signed-off-by: Frédéric Pétrot <[email protected]>
> ---

This patch adds style issues that you're fixing with patch 3:

$ ./scripts/checkpatch.pl 0002-disas-riscv.c-Correct-disasm-of-lq.patch
ERROR: suspect code indent for conditional statements (15, 18)
#46: FILE: disas/riscv.c:2899:
+               if ((inst >> 7) & 0b11111) {
+                  op = rv_op_lq;

ERROR: trailing statements should be on next line
#50: FILE: disas/riscv.c:2903:
+                  case 0: op = rv_op_cbo_inval; break;

ERROR: trailing statements should be on next line
#51: FILE: disas/riscv.c:2904:
+                  case 1: op = rv_op_cbo_clean; break;

ERROR: trailing statements should be on next line
#52: FILE: disas/riscv.c:2905:
+                  case 2: op = rv_op_cbo_flush; break;

ERROR: trailing statements should be on next line
#53: FILE: disas/riscv.c:2906:
+                  case 4: op = rv_op_cbo_zero; break;

total: 5 errors, 0 warnings, 39 lines checked


What we want to do is to squash patch 3 into this patch to not have the style
issues being added in the first place.


Assuming patch 3 is squashed into this one:


Reviewed-by: Daniel Henrique Barboza <[email protected]>



>   disas/riscv.c              | 19 ++++++++++---------
>   target/riscv/insn32.decode |  2 ++
>   2 files changed, 12 insertions(+), 9 deletions(-)
> 
> diff --git a/disas/riscv.c b/disas/riscv.c
> index 490bda8e56..831ff526ad 100644
> --- a/disas/riscv.c
> +++ b/disas/riscv.c
> @@ -2889,21 +2889,22 @@ static void decode_inst_opcode(rv_decode *dec, rv_isa isa)
>               case 2:
>                  /*
>                   * 'lq' shares the "(...) 010 ..... 0001111" opcode space
> -                * with 'cbo' insns.
> +                * with 'cbo' insns, but assumes rd != 0.
>                   *
>                   * cbo_inval  0000000 00000 ..... 010 00000 0001111
>                   * cbo_clean  0000000 00001 ..... 010 00000 0001111
>                   * cbo_flush  0000000 00010 ..... 010 00000 0001111
>                   * cbo_zero   0000000 00100 ..... 010 00000 0001111
> -                *
> -                * Anything that doesn't match these will default to 'lq'.
>                   */
> -               switch (inst >> 20) {
> -               case 0: op = rv_op_cbo_inval; break;
> -               case 1: op = rv_op_cbo_clean; break;
> -               case 2: op = rv_op_cbo_flush; break;
> -               case 4: op = rv_op_cbo_zero; break;
> -               default: op = rv_op_lq; break;
> +               if ((inst >> 7) & 0b11111) {
> +                  op = rv_op_lq;
> +               } else {
> +                  switch (inst >> 20) {
> +                  case 0: op = rv_op_cbo_inval; break;
> +                  case 1: op = rv_op_cbo_clean; break;
> +                  case 2: op = rv_op_cbo_flush; break;
> +                  case 4: op = rv_op_cbo_zero; break;
> +                  }
>                  }
>               }
>               break;
> diff --git a/target/riscv/insn32.decode b/target/riscv/insn32.decode
> index 21272fdb50..aa02dae3c9 100644
> --- a/target/riscv/insn32.decode
> +++ b/target/riscv/insn32.decode
> @@ -216,6 +216,8 @@ ldu      ............   ..... 111 ..... 0000011 @i
>     ]
>   
>     # *** RVI128 lq ***
> +  # *** Catches an lq with rd = 0, which we disallow
> +  illegal  ------------   ----- 010 00000 0001111
>     lq       ............   ..... 010 ..... 0001111 @i
>   }
>   sq       ............   ..... 100 ..... 0100011 @s
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.