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

Thanks Daniel，Chao and Nutty for the review.

I’ll address these issues in v3 by:

  * adding a descriptive commit body to each patch and comments
    for functions that need further explanation;
  * aligning the continuation backslashes in the helper macros;
  * reusing the existing |EXTRACT*|definitions in patch 02/18 instead of
    introducing duplicate definitions; and
  * investigating the addition of P-extension test cases to the QEMU TCG
    test suite, depending on the available toolchain support.

I’ll also document these changes in the v3 cover letter.

Best regards,
Molly

在 2026/8/6 2:36, Daniel Henrique Barboza 写道:
>
>
> 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
>>>