Re: [PATCH v3 01/17] riscv: Introduce instruction table generation

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Medium] Zcmp compressed instructions (e.g., `cm.*`) are incorrectly generated with 32-bit sizes instead of 16-bit.
- [Low] Unsupported instruction extraction stubs lack a return statement, causing `-Wreturn-type` build errors.
--

--- Patch [1]: [PATCH v3 01/17] riscv: Introduce instruction table generation ---
commit 893d119e65d6edd9ce9e98809ffad3450c8d6a8f
Author: Charlie Jenkins <thecharlesjenkins@gmail.com>

riscv: Introduce instruction table generation

This commit introduces a shell script to auto-generate a header from an
instruction table for RISC-V. It aims to replace hand-written instruction
macros.

Link: https://github.com/riscv-software-src/riscv-unified-db

> diff --git a/arch/riscv/tools/insn_tbl.sh b/arch/riscv/tools/insn_tbl.sh
> new file mode 100755
> index 0000000000000..f09b93951cde5
> --- /dev/null
> +++ b/arch/riscv/tools/insn_tbl.sh
> @@ -0,0 +1,258 @@

[ ... ]

> +        non_compressed_insn=${name##c.*}
> +        invalid_inst_functions=""
> +        variable_params=""
> +        constraints=""
> +        match=""
> +        mask=""
> +        make=""
> +
> +        # All compressed instructions start with "c."
> +        size=${non_compressed_insn:+32};
> +        size=${size:-16};

[Severity: Medium]
Will this correctly identify Zcmp compressed instructions like cm.pop?

Since they start with "cm." instead of "c.", the glob pattern c.* won't
match them, assigning them a 32-bit size instead of a 16-bit size. Could this
cause the generated insert and extract helpers to wrongly expect u32 *
instead of u16 *?

[ ... ]

> +            printf "static __always_inline void riscv_insn_${formatted_inst_name}_insert_${variable_name}(u${size} *${insn}, ${type}32 ${var})\n"
> +            printf "{\n"
> +            printf "\t*_insn &= ${insert_mask# & };\n"
> +            printf "\t*_insn |= ${insert# | };\n"
> +            printf "}\n"
> +
> +            if [ "${only_base}" ]; then
> +                invalid_inst_functions="${invalid_inst_functions}static __always_inline ${type}${size} riscv_insn_${formatted_inst_name}_extract_${variable_name}(u${size} ${insn}) {\n\tBUILD_BUG_ON_MSG(1, \"${name} is not supported on non ${only_base}-bit systems.\");\n}\n"
> +            fi

[Severity: Low]
Do these unsupported instruction extraction stubs need a return statement?

Since they have non-void return types, omitting a return could cause a build
failure due to -Werror=return-type on supported architectures like RV32.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260707-riscv_insn_table-v3-0-3f8e5f75ba97@gmail.com?part=1