Re: [PATCH v2 01/16] riscv: Introduce instruction table generation

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 4 potential issue(s) to consider:
- [High] The shell script incorrectly truncates multiple instruction constraints during bash parameter expansion, causing generated matchers to drop all but the first constraint.
- [High] Extracted immediate values for compressed instructions are incorrectly truncated to 16 bits, corrupting instructions with large shifted immediates like c.lui.
- [Low] The generated header relies on `sign_extend32` but fails to include `<linux/bitops.h>`.
- [Low] Signed integer shift overflow when generating instruction masks and matches.
--

commit 2e4d01f18f465b8609784ae2ae57f5539bddd357
Author: Charlie Jenkins <thecharlesjenkins@gmail.com>

riscv: Introduce instruction table generation

Eliminate the need to hand-write riscv instructions by using a shell
script to autogenerate a header from an instruction table. This is modeled
after the syscall table infrastructure. The table is generated externally
by riscv-unified-db.

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..aede14c80146f
> --- /dev/null
> +++ b/arch/riscv/tools/insn_tbl.sh
> @@ -0,0 +1,256 @@

[ ... ]

> +echo "#ifndef RISCV_INSN_GEN_H" >> $outfile
> +echo "#define RISCV_INSN_GEN_H" >> $outfile
> +echo >> $outfile
> +
> +printf "#include <linux/bits.h>" >> $outfile
> +echo >> $outfile

[Severity: Low]
The generated header relies on sign_extend32() for signed fields, but only
includes <linux/bits.h>. Could this missing <linux/bitops.h> inclusion cause
compilation failures for future standalone inclusions of the generated header?

[ ... ]

> +            if [ ${offset} -gt 0 ]; then
> +                s=" << ${offset}"
> +            else
> +                s=""
> +            fi
> +
> +            mask="${mask}${s}"
> +
> +            match="${match} | 0b${bits}${s}"

[Severity: Low]
Does this code trigger signed integer shift overflow warnings? The script
generates macros using signed binary literals shifted by large amounts
(e.g., 0b111000100000 << 20 for fclass.d). Even with -fno-strict-overflow,
this can cause compiler warnings if -Wshift-overflow is enabled. Would it be
better to append a U suffix to these binary literals to ensure unsigned
shifts?

[ ... ]

> +            if [ "${variable}" != "${variable#*!}" ]; then
> +                raw_constraints="${variable#*!}"
> +                raw_constraints="${raw_constraints%%[=<~!]**}"

[Severity: High]
Does this parameter expansion inadvertently truncate multiple constraints?
Because the ! character is included in the character class [=<~!], the
%% operator greedily matches the longest suffix starting with the first !
it encounters.

For example, with c.ldsp which has constraints like xd!0!1!3!5!7=11-7, it
looks like this strips everything after the first !, leaving only 0. This
would cause the generated matcher to only verify that xd != 0 and fail to
enforce the rest of the constraints.

[ ... ]

> +            if [ ${sign_extend} ]; then
> +                extract="sign_extend32(${extract}, $((offset + left_shift - 1)))"
> +                type="s"
> +            else
> +                type="u"
> +            fi
> +
> +            printf "static __always_inline ${type}${size} riscv_insn_${formatted_inst_name}_extract_${variable_name}(u${size} ${insn})\n"

[Severity: High]
Does this return type cause unintended truncation for compressed instructions
with large immediates?

The script deduces the return type as s16 or u16 for compressed instructions.
However, instructions like c.lui encode a 6-bit immediate shifted left by 12,
resulting in an 18-bit signed value. It appears the generated function correctly
computes the 32-bit signed value, but returning it as s16 silently truncates
the upper 16 bits (including the sign bit). Could this cause data corruption
when decoding such instructions?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260621-riscv_insn_table-v2-0-7f0810ee2274@gmail.com?part=1