Re: [PING][PATCH v2] gdb/record: Support fense.tso, zicond, bitmanip, sinval and zihintntl in riscv

[Guinevere Larsen <guinevere@redhat.com>]

On 10/20/25 8:39 AM, timurgol007 wrote:
> Added record-full support for these extensions. As some opcodes were
> missed in riscv-opc.h, added them there using riscv-opcodes repo.
>
> Hi! Polite ping. I'm specifying Andrew Burgess as the recipient because
> it seems like he's the only one who can approve this from RISC-V side.
> ---

Hi!

I can't really review the specific risc-v changes, as I'm not familiar 
with the architecture, but I do know that this patch is now outdated, 
once the "Speeding up RISC-V recording" patch has been merged, since 
"set_ordinary_record_type" no longer exists, and there is a conflict 
because both patches change need_save_pc to no longer be static.

As far as I can see, this patch looks OK, so I'm fine with giving a 
review tag

Reviewed-By: Guinevere Larsen <guinevere@redhat.com>

I'll give this some more time for Andrew or someone with more RISC-V 
knowledge to look over, rather than just saying you can push.

>   gdb/riscv-tdep.c           | 85 ++++++++++++++++++++++++++++++++++++--
>   include/opcode/riscv-opc.h | 43 +++++++++++++++++++
>   2 files changed, 125 insertions(+), 3 deletions(-)
>
> diff --git a/gdb/riscv-tdep.c b/gdb/riscv-tdep.c
> index 697071bf69e..46eb4bf94f8 100644
> --- a/gdb/riscv-tdep.c
> +++ b/gdb/riscv-tdep.c
> @@ -5076,7 +5076,15 @@ class riscv_recorded_insn final
>   	   || is_bge_insn (ival) || is_bltu_insn (ival) || is_bgeu_insn (ival)
>   	   || is_fence_insn (ival) || is_pause_insn (ival)
>   	   || is_fence_i_insn (ival) || is_wfi_insn (ival)
> -	   || is_sfence_vma_insn (ival));
> +	   || is_sfence_vma_insn (ival) || is_fence_tso_insn (ival)
> +	   /* svinval  */
> +	   || is_sfence_inval_ir_insn (ival) || is_sfence_w_inval_insn (ival)
> +	   || is_sinval_vma_insn (ival) || is_hinval_gvma_insn (ival)
> +	   || is_hinval_vvma_insn (ival) || is_hfence_gvma_insn (ival)
> +	   || is_hfence_vvma_insn (ival)
> +	   /* zihintntl  */
> +	   || is_ntl_p1_insn (ival) || is_ntl_pall_insn (ival)
> +	   || is_ntl_s1_insn (ival) || is_ntl_all_insn (ival));
>     }
>
>     /* Returns true if instruction is classified.  */
> @@ -5090,7 +5098,7 @@ class riscv_recorded_insn final
>     }
>
>     /* Returns true if instruction needs only saving pc and rd.  */
> -  static bool
> +  bool
>     need_save_pc_rd (ULONGEST ival) noexcept
>     {
>       return (is_lui_insn (ival) || is_auipc_insn (ival) || is_jal_insn (ival)
> @@ -5123,7 +5131,47 @@ class riscv_recorded_insn final
>   	   || is_flt_d_insn (ival) || is_fle_d_insn (ival)
>   	   || is_fclass_d_insn (ival) || is_fcvt_w_d_insn (ival)
>   	   || is_fcvt_wu_d_insn (ival) || is_fcvt_l_d_insn (ival)
> -	   || is_fcvt_lu_d_insn (ival) || is_fmv_x_d_insn (ival));
> +	   || is_fcvt_lu_d_insn (ival) || is_fmv_x_d_insn (ival)
> +	   /* zicond  */
> +	   || is_czero_eqz_insn (ival) || is_czero_nez_insn (ival)
> +	   /* bitmanip  */
> +	   || (m_xlen == 8 && is_add_uw_insn (ival)) || is_andn_insn (ival)
> +	   || is_bclr_insn (ival) || (m_xlen == 8 && is_bclri_insn (ival))
> +	   || (m_xlen == 4 && is_bclri_rv32_insn (ival)) || is_bext_insn (ival)
> +	   || (m_xlen == 8 && is_bexti_insn (ival))
> +	   || (m_xlen == 4 && is_bexti_rv32_insn (ival)) || is_binv_insn (ival)
> +	   || (m_xlen == 8 && is_binvi_insn (ival))
> +	   || (m_xlen == 4 && is_binvi_rv32_insn (ival))
> +	   || is_brev8_insn (ival) || is_bset_insn (ival)
> +	   || (m_xlen == 8 && is_bseti_insn (ival))
> +	   || (m_xlen == 4 && is_bseti_rv32_insn (ival))
> +	   || is_clmul_insn (ival) || is_clmulh_insn (ival)
> +	   || is_clmulr_insn (ival) || is_clz_insn (ival)
> +	   || (m_xlen == 8 && is_clzw_insn (ival)) || is_cpop_insn (ival)
> +	   || (m_xlen == 8 && is_cpopw_insn (ival)) || is_ctz_insn (ival)
> +	   || (m_xlen == 8 && is_ctzw_insn (ival)) || is_max_insn (ival)
> +	   || is_maxu_insn (ival) || is_min_insn (ival) || is_minu_insn (ival)
> +	   || is_orc_b_insn (ival) || is_orn_insn (ival) || is_pack_insn (ival)
> +	   || is_packh_insn (ival) || (m_xlen == 8 && is_packw_insn (ival))
> +	   || (m_xlen == 8 && is_rev8_insn (ival))
> +	   || (m_xlen == 4 && is_rev8_rv32_insn (ival)) || is_rol_insn (ival)
> +	   || (m_xlen == 8 && is_rolw_insn (ival)) || is_ror_insn (ival)
> +	   || (m_xlen == 8 && is_rori_insn (ival))
> +	   || (m_xlen == 4 && is_rori_rv32_insn (ival))
> +	   || (m_xlen == 8 && is_roriw_insn (ival))
> +	   || (m_xlen == 8 && is_rorw_insn (ival)) || is_sext_b_insn (ival)
> +	   || is_sext_h_insn (ival) || is_sh1add_insn (ival)
> +	   || (m_xlen == 8 && is_sh1add_uw_insn (ival))
> +	   || is_sh2add_insn (ival)
> +	   || (m_xlen == 8 && is_sh2add_uw_insn (ival))
> +	   || is_sh3add_insn (ival)
> +	   || (m_xlen == 8 && is_sh3add_uw_insn (ival))
> +	   || (m_xlen == 8 && is_slli_uw_insn (ival))
> +	   || (m_xlen == 4 && is_unzip_insn (ival)) || is_xnor_insn (ival)
> +	   || is_xperm4_insn (ival) || is_xperm8_insn (ival)
> +	   || is_zext_h_insn (ival)
> +	   || (m_xlen == 4 && is_zext_h_rv32_insn (ival))
> +	   || (m_xlen == 4 && is_zip_insn (ival)));
>     }
>
>     /* Returns true if instruction is classified.  This function can set
> @@ -5416,6 +5464,37 @@ class riscv_recorded_insn final
>   	       || !save_mem (addr + offset, 4) || set_ordinary_record_type ());
>         }
>
> +    /* c.zihintntl  */
> +    if (is_c_ntl_p1_insn (ival) || is_c_ntl_pall_insn (ival)
> +	|| is_c_ntl_s1_insn (ival) || is_c_ntl_all_insn (ival))
> +      return set_ordinary_record_type ();
> +
> +    /* c.bitmanip  */
> +    if (is_c_lbu_insn (ival) || is_c_lhu_insn (ival) || is_c_lh_insn (ival))
> +      return (!save_reg (decode_crs2_short (ival))
> +	     || set_ordinary_record_type ());
> +
> +    if (is_c_sb_insn (ival))
> +      {
> +	ULONGEST offset = ULONGEST{EXTRACT_ZCB_BYTE_UIMM (ival)};
> +	return (!read_reg (regcache, decode_crs1_short (ival), addr)
> +	       || !save_mem (addr + offset, 1) || set_ordinary_record_type ());
> +      }
> +
> +    if (is_c_sh_insn (ival))
> +      {
> +	ULONGEST offset = ULONGEST{EXTRACT_ZCB_HALFWORD_UIMM (ival)};
> +	return (!read_reg (regcache, decode_crs1_short (ival), addr)
> +	       || !save_mem (addr + offset, 2) || set_ordinary_record_type ());
> +      }
> +
> +    if (is_c_zext_b_insn (ival) || is_c_sext_b_insn (ival)
> +       || is_c_zext_h_insn (ival) || is_c_sext_h_insn (ival)
> +       || is_c_not_insn (ival) || is_c_mul_insn (ival)
> +       || (m_xlen == 8 && is_c_zext_w_insn (ival)))
> +      return (!save_reg (decode_crs1_short (ival))
> +	     || set_ordinary_record_type ());
> +
>       warning (_("Currently this instruction with len 2(%s) is unsupported"),
>   	     hex_string (ival));
>       return false;
> diff --git a/include/opcode/riscv-opc.h b/include/opcode/riscv-opc.h
> index 1c649628390..6a5d54b1615 100644
> --- a/include/opcode/riscv-opc.h
> +++ b/include/opcode/riscv-opc.h
> @@ -579,6 +579,32 @@
>   #define MASK_RORW  0xfe00707f
>   #define MATCH_RORIW 0x6000501b
>   #define MASK_RORIW  0xfe00707f
> +#define MATCH_BCLRI_RV32 0x48001013
> +#define MASK_BCLRI_RV32 0xfe00707f
> +#define MATCH_BEXTI_RV32 0x48005013
> +#define MASK_BEXTI_RV32 0xfe00707f
> +#define MATCH_BINVI_RV32 0x68001013
> +#define MASK_BINVI_RV32 0xfe00707f
> +#define MATCH_BREV8 0x68705013
> +#define MASK_BREV8 0xfff0707f
> +#define MATCH_BSETI_RV32 0x28001013
> +#define MASK_BSETI_RV32 0xfe00707f
> +#define MATCH_ORC_B 0x28705013
> +#define MASK_ORC_B 0xfff0707f
> +#define MATCH_REV8 0x6b805013
> +#define MASK_REV8 0xfff0707f
> +#define MATCH_REV8_RV32 0x69805013
> +#define MASK_REV8_RV32 0xfff0707f
> +#define MATCH_RORI_RV32 0x60005013
> +#define MASK_RORI_RV32 0xfe00707f
> +#define MATCH_UNZIP 0x8f05013
> +#define MASK_UNZIP 0xfff0707f
> +#define MATCH_ZEXT_H 0x800403b
> +#define MASK_ZEXT_H 0xfff0707f
> +#define MATCH_ZEXT_H_RV32 0x8004033
> +#define MASK_ZEXT_H_RV32 0xfff0707f
> +#define MATCH_ZIP 0x8f01013
> +#define MASK_ZIP 0xfff0707f
>   #define MATCH_SH1ADD 0x20002033
>   #define MASK_SH1ADD  0xfe00707f
>   #define MATCH_SH2ADD 0x20004033
> @@ -2269,6 +2295,8 @@
>   #define MASK_C_SEXT_H 0xfc7f
>   #define MATCH_C_ZEXT_W 0x9c71
>   #define MASK_C_ZEXT_W 0xfc7f
> +#define MATCH_C_SEXT_W 0x2001
> +#define MASK_C_SEXT_W 0xf07f
>   #define MATCH_C_NOT 0x9c75
>   #define MASK_C_NOT 0xfc7f
>   #define MATCH_C_MUL 0x9c41
> @@ -4370,6 +4398,7 @@ DECLARE_INSN(sd, MATCH_SD, MASK_SD)
>   DECLARE_INSN(pause, MATCH_PAUSE, MASK_PAUSE)
>   DECLARE_INSN(fence, MATCH_FENCE, MASK_FENCE)
>   DECLARE_INSN(fence_i, MATCH_FENCE_I, MASK_FENCE_I)
> +DECLARE_INSN(fence_tso, MATCH_FENCE_TSO, MASK_FENCE_TSO)
>   DECLARE_INSN(mul, MATCH_MUL, MASK_MUL)
>   DECLARE_INSN(mulh, MATCH_MULH, MASK_MULH)
>   DECLARE_INSN(mulhsu, MATCH_MULHSU, MASK_MULHSU)
> @@ -4570,6 +4599,19 @@ DECLARE_INSN(cpopw, MATCH_CPOPW, MASK_CPOPW)
>   DECLARE_INSN(rolw, MATCH_ROLW, MASK_ROLW)
>   DECLARE_INSN(rorw, MATCH_RORW, MASK_RORW)
>   DECLARE_INSN(roriw, MATCH_RORIW, MASK_RORIW)
> +DECLARE_INSN(bclri_rv32, MATCH_BCLRI_RV32, MASK_BCLRI_RV32)
> +DECLARE_INSN(bexti_rv32, MATCH_BEXTI_RV32, MASK_BEXTI_RV32)
> +DECLARE_INSN(binvi_rv32, MATCH_BINVI_RV32, MASK_BINVI_RV32)
> +DECLARE_INSN(brev8, MATCH_BREV8, MASK_BREV8)
> +DECLARE_INSN(bseti_rv32, MATCH_BSETI_RV32, MASK_BSETI_RV32)
> +DECLARE_INSN(orc_b, MATCH_ORC_B, MASK_ORC_B)
> +DECLARE_INSN(rev8, MATCH_REV8, MASK_REV8)
> +DECLARE_INSN(rev8_rv32, MATCH_REV8_RV32, MASK_REV8_RV32)
> +DECLARE_INSN(rori_rv32, MATCH_RORI_RV32, MASK_RORI_RV32)
> +DECLARE_INSN(unzip, MATCH_UNZIP, MASK_UNZIP)
> +DECLARE_INSN(zext_h, MATCH_ZEXT_H, MASK_ZEXT_H)
> +DECLARE_INSN(zext_h_rv32, MATCH_ZEXT_H_RV32, MASK_ZEXT_H_RV32)
> +DECLARE_INSN(zip, MATCH_ZIP, MASK_ZIP)
>   DECLARE_INSN(sh1add, MATCH_SH1ADD, MASK_SH1ADD)
>   DECLARE_INSN(sh2add, MATCH_SH2ADD, MASK_SH2ADD)
>   DECLARE_INSN(sh3add, MATCH_SH3ADD, MASK_SH3ADD)
> @@ -4803,6 +4845,7 @@ DECLARE_INSN(vsm3me_vv, MATCH_VSM3ME_VV, MASK_VSM3ME_VV)
>   /* Zcb instructions.  */
>   DECLARE_INSN(c_sext_b, MATCH_C_SEXT_B, MASK_C_SEXT_B)
>   DECLARE_INSN(c_sext_h, MATCH_C_SEXT_H, MASK_C_SEXT_H)
> +DECLARE_INSN(c_sext_w, MATCH_C_SEXT_W, MASK_C_SEXT_W)
>   DECLARE_INSN(c_zext_b, MATCH_C_ZEXT_B, MASK_C_ZEXT_B)
>   DECLARE_INSN(c_zext_h, MATCH_C_ZEXT_H, MASK_C_ZEXT_H)
>   DECLARE_INSN(c_zext_w, MATCH_C_ZEXT_W, MASK_C_ZEXT_W)
> --
> 2.34.1
>

-- 
Cheers,
Guinevere Larsen
It/she