Re: [PATCH v2 00/18] target/riscv: Add support for RISC-V P

On 7/29/2026 6:32 AM, Chao Liu wrote:
> On Fri, Jul 17, 2026 at 10:06:53AM +0800, Molly Chen wrote:
>> This series adds support for the RISC-V Packed SIMD (P) extension.
>>
>> The P extension defines packed-SIMD fixed-point operations intended
>> for DSP-style workloads such as multimedia and signal processing.
>> These instructions operate on packed subword elements within
>> general-purpose registers (GPRs).
>>
>> The implementation follows the current development draft of the
>> specification (v0.20):
>>
>>    https://github.com/riscv/riscv-p-spec/
>>    or
>>    https://www.jhauser.us/RISCV/ext-P/
>>
>> Compared with v1, this version reduces implementation boilerplate by
>> defining and reusing helper macros for common patterns in
>> trans_rvp.c.inc and psimd_helper.c. It also consolidates the P
>> extension prerequisite validation in riscv_cpu_validate_p().
>>
>> The Zbkb dependency has been removed to align the implementation with
>> the current specification. This version also implements the
>> specification-defined control of vxsat, which was missing from v1.
>>
>> All instructions have been functionally tested using the following
>> test suite:
>>
>>    https://github.com/mollybuild/qemu-riscv-test-uart
>>
>> The implementation focuses on functional correctness and ISA
>> coverage. Performance optimizations were not considered at this
>> stage.
>>
>> Feedback on the implementation details would be highly appreciated.
>>
>> Tested with:
>>
>>    - ninja -C build test
>>    - make -C build check-qtest-riscv32
>>    - make -C build check-qtest-riscv64
>>    - Functional tests for all P extension instructions using
>>      qemu-riscv-test-uart
>>
>> No regressions or test failures were observed.
>>
>> ---
>> Changes in v2:
>>
>>    - Refactored trans_rvp.c.inc and psimd_helper.c by defining and
>>      reusing helper macros for common implementation patterns, reducing
>>      boilerplate and overall code size.
>>    - Moved the P extension prerequisite checks into
>>      riscv_cpu_validate_p().
>>    - Removed the Zbkb dependency to align with the current
>>      specification.
>>    - Added the specification-defined vxsat control mechanism, which was
>>      missing from v1.
>>
> This patch set is a significant improvement over v1. I think there are still
> a few common issues we can address together:

v2 is a huge improvement indeed.  v1 had 11k lines, v2 cut those by half due to
the smart use of helpers.

> 
> 1. The line break characters '\' in the helper macros we defined need to be space-aligned.
> 2. Every patch is currently missing a commit body.
> 
> I think these changes are quite necessary.

Yeah, having a commit message allows us to make fewer guesses when reviewing code.
E.g. the usage of __int128t__ macros and the choice of some rounding formulas.  We
can make review  assumptions and hope all is fine, but it's not ideal.

And, as Chao mentioned in his other reply, having test cases will be helpful to
assert that everything being added here is sane.  In particular with the amount
of lines we're adding here.


Cheers,
Daniel

> 
> Thanks,
> Chao
>> v1:
>>    https://lists.nongnu.org/archive/html/qemu-riscv/2026-04/msg00301.html
>>
>> Molly Chen (18):
>>    target/riscv: Add packed SIMD extension state
>>    target/riscv: Add packed SIMD helper framework
>>    target/riscv: Add packed SIMD arithmetic instructions
>>    target/riscv: Add packed SIMD averaging and rounding instructions
>>    target/riscv: Add packed SIMD absolute, difference, compare and mask
>>      instructions
>>    target/riscv: Add packed SIMD shift instructions
>>    target/riscv: Add packed SIMD exchange instructions
>>    target/riscv: Add packed SIMD horizontal reduction instructions
>>    target/riscv: Add packed SIMD pack, merge and count-leading
>>      instructions
>>    target/riscv: Add packed SIMD multiplication instructions
>>    target/riscv: Add packed SIMD multiply-accumulate instructions
>>    target/riscv: Add packed SIMD Q-format multiplication instructions
>>    target/riscv: Add packed SIMD Q-format MAC instructions
>>    target/riscv: Add packed SIMD two-way MAC instructions
>>    target/riscv: Add packed SIMD four-way MAC instructions
>>    target/riscv: Add packed SIMD load-replicate instructions
>>    target/riscv: Add packed SIMD RV32 only instructions
>>    target/riscv: Remove Zbkb dependency from P extension to align with
>>      the spec
>>
>>   target/riscv/cpu.c                          |    5 +-
>>   target/riscv/cpu.h                          |    8 +
>>   target/riscv/cpu_bits.h                     |    2 +
>>   target/riscv/helper.h                       |  529 ++++
>>   target/riscv/insn32.decode                  |  829 +++++
>>   target/riscv/machine.c                      |   19 +
>>   target/riscv/tcg/csr.c                      |   39 +-
>>   target/riscv/tcg/insn_trans/trans_rvb.c.inc |    4 +-
>>   target/riscv/tcg/insn_trans/trans_rvp.c.inc | 1389 ++++++++
>>   target/riscv/tcg/meson.build                |    3 +-
>>   target/riscv/tcg/psimd_helper.c             | 3165 +++++++++++++++++++
>>   target/riscv/tcg/tcg-cpu.c                  |   46 +
>>   target/riscv/tcg/translate.c                |    6 +
>>   13 files changed, 6038 insertions(+), 6 deletions(-)
>>   create mode 100644 target/riscv/tcg/insn_trans/trans_rvp.c.inc
>>   create mode 100644 target/riscv/tcg/psimd_helper.c
>>
>>
>> base-commit: 8fb307591625731060d399aca42373c02c7cb040
>> -- 
>> 2.34.1
>>