Re: [PATCH 40/56] disas/riscv: Return rv_opcode_data pointer from decoders

On Sun, 2026-08-09 at 15:35 -0700, Richard Henderson wrote:
> Rather than putting array + index into rv_decode,
> return the pointer to the object directly.
> 
> Signed-off-by: Richard Henderson <[email protected]>

Reviewed-by: Alistair Francis <[email protected]>

Alistair

> ---
>  disas/riscv-xlrbr.h    |  2 +-
>  disas/riscv-xthead.h   | 22 ++++++++---------
>  disas/riscv-xventana.h |  2 +-
>  disas/riscv.h          |  2 --
>  disas/riscv-xlrbr.c    |  5 ++--
>  disas/riscv-xthead.c   | 55 +++++++++++++++++-----------------------
> --
>  disas/riscv-xventana.c |  5 ++--
>  disas/riscv.c          | 23 +++++++-----------
>  8 files changed, 48 insertions(+), 68 deletions(-)
> 
> diff --git a/disas/riscv-xlrbr.h b/disas/riscv-xlrbr.h
> index ebf3d4c133..da14f1b340 100644
> --- a/disas/riscv-xlrbr.h
> +++ b/disas/riscv-xlrbr.h
> @@ -12,6 +12,6 @@
>  
>  #include "disas/riscv.h"
>  
> -void decode_xlrbr(rv_decode *, rv_isa);
> +const rv_opcode_data *decode_xlrbr(rv_decode *, rv_isa);
>  
>  #endif /* DISAS_RISCV_XLRBR_H */
> diff --git a/disas/riscv-xthead.h b/disas/riscv-xthead.h
> index 0c65c8e0af..f2160be8a4 100644
> --- a/disas/riscv-xthead.h
> +++ b/disas/riscv-xthead.h
> @@ -11,16 +11,16 @@
>  
>  #include "disas/riscv.h"
>  
> -void decode_xtheadba(rv_decode *, rv_isa);
> -void decode_xtheadbb(rv_decode *, rv_isa);
> -void decode_xtheadbs(rv_decode *, rv_isa);
> -void decode_xtheadcmo(rv_decode *, rv_isa);
> -void decode_xtheadcondmov(rv_decode *, rv_isa);
> -void decode_xtheadfmemidx(rv_decode *, rv_isa);
> -void decode_xtheadfmv(rv_decode *, rv_isa);
> -void decode_xtheadmac(rv_decode *, rv_isa);
> -void decode_xtheadmemidx(rv_decode *, rv_isa);
> -void decode_xtheadmempair(rv_decode *, rv_isa);
> -void decode_xtheadsync(rv_decode *, rv_isa);
> +const rv_opcode_data *decode_xtheadba(rv_decode *, rv_isa);
> +const rv_opcode_data *decode_xtheadbb(rv_decode *, rv_isa);
> +const rv_opcode_data *decode_xtheadbs(rv_decode *, rv_isa);
> +const rv_opcode_data *decode_xtheadcmo(rv_decode *, rv_isa);
> +const rv_opcode_data *decode_xtheadcondmov(rv_decode *, rv_isa);
> +const rv_opcode_data *decode_xtheadfmemidx(rv_decode *, rv_isa);
> +const rv_opcode_data *decode_xtheadfmv(rv_decode *, rv_isa);
> +const rv_opcode_data *decode_xtheadmac(rv_decode *, rv_isa);
> +const rv_opcode_data *decode_xtheadmemidx(rv_decode *, rv_isa);
> +const rv_opcode_data *decode_xtheadmempair(rv_decode *, rv_isa);
> +const rv_opcode_data *decode_xtheadsync(rv_decode *, rv_isa);
>  
>  #endif /* DISAS_RISCV_XTHEAD_H */
> diff --git a/disas/riscv-xventana.h b/disas/riscv-xventana.h
> index 1006c3be00..d28e84c7e4 100644
> --- a/disas/riscv-xventana.h
> +++ b/disas/riscv-xventana.h
> @@ -11,6 +11,6 @@
>  
>  #include "disas/riscv.h"
>  
> -void decode_xventanacondops(rv_decode*, rv_isa);
> +const rv_opcode_data *decode_xventanacondops(rv_decode*, rv_isa);
>  
>  #endif /* DISAS_RISCV_XVENTANA_H */
> diff --git a/disas/riscv.h b/disas/riscv.h
> index ab62c0db94..241dd47d7f 100644
> --- a/disas/riscv.h
> +++ b/disas/riscv.h
> @@ -182,10 +182,8 @@ typedef struct {
>      const RISCVCPUConfig *cfg;
>      uint64_t  pc;
>      uint64_t  inst;
> -    const rv_opcode_data *opcode_data;
>      int32_t   imm;
>      int32_t   imm1;
> -    uint16_t  op;
>      uint8_t   rd;
>      uint8_t   rs1;
>      uint8_t   rs2;
> diff --git a/disas/riscv-xlrbr.c b/disas/riscv-xlrbr.c
> index d0092e9edd..b12916c9da 100644
> --- a/disas/riscv-xlrbr.c
> +++ b/disas/riscv-xlrbr.c
> @@ -36,7 +36,7 @@ static const rv_opcode_data xlrbr_opcode_data[] = {
>      { "crc32c.d", rv_codec_r, rv_fmt_rd_rs1 },
>  };
>  
> -void decode_xlrbr(rv_decode *dec, rv_isa isa)
> +const rv_opcode_data *decode_xlrbr(rv_decode *dec, rv_isa isa)
>  {
>      rv_inst inst = dec->inst;
>      rv_opcode op = rv_op_illegal;
> @@ -76,6 +76,5 @@ void decode_xlrbr(rv_decode *dec, rv_isa isa)
>          break;
>      }
>  
> -    dec->opcode_data = xlrbr_opcode_data;
> -    dec->op = op;
> +    return op == rv_op_illegal ? NULL : &xlrbr_opcode_data[op];
>  }
> diff --git a/disas/riscv-xthead.c b/disas/riscv-xthead.c
> index 757d6ff39f..428a6380e9 100644
> --- a/disas/riscv-xthead.c
> +++ b/disas/riscv-xthead.c
> @@ -246,7 +246,7 @@ static const rv_opcode_data xthead_opcode_data[]
> = {
>      { "th.sync.s", rv_codec_none, rv_fmt_none },
>  };
>  
> -void decode_xtheadba(rv_decode *dec, rv_isa isa)
> +const rv_opcode_data *decode_xtheadba(rv_decode *dec, rv_isa isa)
>  {
>      rv_inst inst = dec->inst;
>      rv_opcode op = rv_op_illegal;
> @@ -272,11 +272,10 @@ void decode_xtheadba(rv_decode *dec, rv_isa
> isa)
>          break;
>      }
>  
> -    dec->opcode_data = xthead_opcode_data;
> -    dec->op = op;
> +    return op == rv_op_illegal ? NULL : &xthead_opcode_data[op];
>  }
>  
> -void decode_xtheadbb(rv_decode *dec, rv_isa isa)
> +const rv_opcode_data *decode_xtheadbb(rv_decode *dec, rv_isa isa)
>  {
>      rv_inst inst = dec->inst;
>      rv_opcode op = rv_op_illegal;
> @@ -331,11 +330,10 @@ void decode_xtheadbb(rv_decode *dec, rv_isa
> isa)
>          break;
>      }
>  
> -    dec->opcode_data = xthead_opcode_data;
> -    dec->op = op;
> +    return op == rv_op_illegal ? NULL : &xthead_opcode_data[op];
>  }
>  
> -void decode_xtheadbs(rv_decode *dec, rv_isa isa)
> +const rv_opcode_data *decode_xtheadbs(rv_decode *dec, rv_isa isa)
>  {
>      rv_inst inst = dec->inst;
>      rv_opcode op = rv_op_illegal;
> @@ -358,11 +356,10 @@ void decode_xtheadbs(rv_decode *dec, rv_isa
> isa)
>          break;
>      }
>  
> -    dec->opcode_data = xthead_opcode_data;
> -    dec->op = op;
> +    return op == rv_op_illegal ? NULL : &xthead_opcode_data[op];
>  }
>  
> -void decode_xtheadcmo(rv_decode *dec, rv_isa isa)
> +const rv_opcode_data *decode_xtheadcmo(rv_decode *dec, rv_isa isa)
>  {
>      rv_inst inst = dec->inst;
>      rv_opcode op = rv_op_illegal;
> @@ -437,11 +434,10 @@ void decode_xtheadcmo(rv_decode *dec, rv_isa
> isa)
>          break;
>      }
>  
> -    dec->opcode_data = xthead_opcode_data;
> -    dec->op = op;
> +    return op == rv_op_illegal ? NULL : &xthead_opcode_data[op];
>  }
>  
> -void decode_xtheadcondmov(rv_decode *dec, rv_isa isa)
> +const rv_opcode_data *decode_xtheadcondmov(rv_decode *dec, rv_isa
> isa)
>  {
>      rv_inst inst = dec->inst;
>      rv_opcode op = rv_op_illegal;
> @@ -465,11 +461,10 @@ void decode_xtheadcondmov(rv_decode *dec,
> rv_isa isa)
>          break;
>      }
>  
> -    dec->opcode_data = xthead_opcode_data;
> -    dec->op = op;
> +    return op == rv_op_illegal ? NULL : &xthead_opcode_data[op];
>  }
>  
> -void decode_xtheadfmemidx(rv_decode *dec, rv_isa isa)
> +const rv_opcode_data *decode_xtheadfmemidx(rv_decode *dec, rv_isa
> isa)
>  {
>      rv_inst inst = dec->inst;
>      rv_opcode op = rv_op_illegal;
> @@ -503,11 +498,10 @@ void decode_xtheadfmemidx(rv_decode *dec,
> rv_isa isa)
>          break;
>      }
>  
> -    dec->opcode_data = xthead_opcode_data;
> -    dec->op = op;
> +    return op == rv_op_illegal ? NULL : &xthead_opcode_data[op];
>  }
>  
> -void decode_xtheadfmv(rv_decode *dec, rv_isa isa)
> +const rv_opcode_data *decode_xtheadfmv(rv_decode *dec, rv_isa isa)
>  {
>      rv_inst inst = dec->inst;
>      rv_opcode op = rv_op_illegal;
> @@ -539,11 +533,10 @@ void decode_xtheadfmv(rv_decode *dec, rv_isa
> isa)
>          break;
>      }
>  
> -    dec->opcode_data = xthead_opcode_data;
> -    dec->op = op;
> +    return op == rv_op_illegal ? NULL : &xthead_opcode_data[op];
>  }
>  
> -void decode_xtheadmac(rv_decode *dec, rv_isa isa)
> +const rv_opcode_data *decode_xtheadmac(rv_decode *dec, rv_isa isa)
>  {
>      rv_inst inst = dec->inst;
>      rv_opcode op = rv_op_illegal;
> @@ -571,11 +564,10 @@ void decode_xtheadmac(rv_decode *dec, rv_isa
> isa)
>          break;
>      }
>  
> -    dec->opcode_data = xthead_opcode_data;
> -    dec->op = op;
> +    return op == rv_op_illegal ? NULL : &xthead_opcode_data[op];
>  }
>  
> -void decode_xtheadmemidx(rv_decode *dec, rv_isa isa)
> +const rv_opcode_data *decode_xtheadmemidx(rv_decode *dec, rv_isa
> isa)
>  {
>      rv_inst inst = dec->inst;
>      rv_opcode op = rv_op_illegal;
> @@ -646,11 +638,10 @@ void decode_xtheadmemidx(rv_decode *dec, rv_isa
> isa)
>          break;
>      }
>  
> -    dec->opcode_data = xthead_opcode_data;
> -    dec->op = op;
> +    return op == rv_op_illegal ? NULL : &xthead_opcode_data[op];
>  }
>  
> -void decode_xtheadmempair(rv_decode *dec, rv_isa isa)
> +const rv_opcode_data *decode_xtheadmempair(rv_decode *dec, rv_isa
> isa)
>  {
>      rv_inst inst = dec->inst;
>      rv_opcode op = rv_op_illegal;
> @@ -681,11 +672,10 @@ void decode_xtheadmempair(rv_decode *dec,
> rv_isa isa)
>          break;
>      }
>  
> -    dec->opcode_data = xthead_opcode_data;
> -    dec->op = op;
> +    return op == rv_op_illegal ? NULL : &xthead_opcode_data[op];
>  }
>  
> -void decode_xtheadsync(rv_decode *dec, rv_isa isa)
> +const rv_opcode_data *decode_xtheadsync(rv_decode *dec, rv_isa isa)
>  {
>      rv_inst inst = dec->inst;
>      rv_opcode op = rv_op_illegal;
> @@ -716,6 +706,5 @@ void decode_xtheadsync(rv_decode *dec, rv_isa
> isa)
>          break;
>      }
>  
> -    dec->opcode_data = xthead_opcode_data;
> -    dec->op = op;
> +    return op == rv_op_illegal ? NULL : &xthead_opcode_data[op];
>  }
> diff --git a/disas/riscv-xventana.c b/disas/riscv-xventana.c
> index 4fb9a29d0b..a4e59312d0 100644
> --- a/disas/riscv-xventana.c
> +++ b/disas/riscv-xventana.c
> @@ -20,7 +20,7 @@ static const rv_opcode_data ventana_opcode_data[] =
> {
>      { "vt.maskcn", rv_codec_r, rv_fmt_rd_rs1_rs2 },
>  };
>  
> -void decode_xventanacondops(rv_decode *dec, rv_isa isa)
> +const rv_opcode_data *decode_xventanacondops(rv_decode *dec, rv_isa
> isa)
>  {
>      rv_inst inst = dec->inst;
>      rv_opcode op = rv_op_illegal;
> @@ -38,6 +38,5 @@ void decode_xventanacondops(rv_decode *dec, rv_isa
> isa)
>          break;
>      }
>  
> -    dec->opcode_data = ventana_opcode_data;
> -    dec->op = op;
> +    return op == rv_op_illegal ? NULL : &ventana_opcode_data[op];
>  }
> diff --git a/disas/riscv.c b/disas/riscv.c
> index 171c60238f..c4b8ee7d07 100644
> --- a/disas/riscv.c
> +++ b/disas/riscv.c
> @@ -2846,7 +2846,7 @@ static const char *csr_name(int csrno)
>  
>  /* decode opcode */
>  
> -static void decode_inst_opcode(rv_decode *dec, rv_isa isa)
> +static const rv_opcode_data *decode_inst_opcode(rv_decode *dec,
> rv_isa isa)
>  {
>      rv_inst inst = dec->inst;
>      rv_opcode op = rv_op_illegal;
> @@ -4541,8 +4541,7 @@ static void decode_inst_opcode(rv_decode *dec,
> rv_isa isa)
>          break;
>      }
>  
> -    dec->opcode_data = rvi_opcode_data;
> -    dec->op = op;
> +    return op == rv_op_illegal ? NULL : &rvi_opcode_data[op];
>  }
>  
>  /* decode operands */
> @@ -5291,14 +5290,12 @@ static GString *disasm_inst(rv_isa isa,
> uint64_t pc, rv_inst inst,
>          .inst = inst,
>          .cfg = cfg,
>      };
> -    const rv_opcode_data *op;
> +    const rv_opcode_data *op = decode_inst_opcode(&dec, isa);
>  
> -    decode_inst_opcode(&dec, isa);
> -
> -    if (dec.op == rv_op_illegal && cfg) {
> +    if (!op && cfg) {
>          static const struct {
>              bool (*guard_func)(const RISCVCPUConfig *);
> -            void (*decode_func)(rv_decode *, rv_isa);
> +            const rv_opcode_data *(*decode_func)(rv_decode *,
> rv_isa);
>          } decoders[] = {
>              { has_xtheadba_p, decode_xtheadba },
>              { has_xtheadbb_p, decode_xtheadbb },
> @@ -5317,19 +5314,17 @@ static GString *disasm_inst(rv_isa isa,
> uint64_t pc, rv_inst inst,
>  
>          for (size_t i = 0; i < ARRAY_SIZE(decoders); i++) {
>              if (decoders[i].guard_func(cfg)) {
> -                decoders[i].decode_func(&dec, isa);
> -                if (dec.op != rv_op_illegal) {
> +                op = decoders[i].decode_func(&dec, isa);
> +                if (op) {
>                      break;
>                  }
>              }
>          }
>      }
> -
> -    if (dec.op == rv_op_illegal) {
> -        dec.opcode_data = rvi_opcode_data;
> +    if (!op) {
> +        op = &rvi_opcode_data[rv_op_illegal];
>      }
>  
> -    op = &dec.opcode_data[dec.op];
>      decode_inst_operands(&dec, isa, op);
>      op = decode_inst_lift_pseudo(&dec, op);
>      return format_inst(24, &dec, op);