Hi Jeff and Pan,

I'd like to elaborate on the motivation for using the HX iterator instead of 
HISI.
According to the RISC-V specification, the pack instruction strictly operates 
on XLEN/2 bits. The HISI iterator unconditionally expands to [HI, SI], which 
means on RV64 targets, GCC would unnecessarily generate and maintain dead RTL 
matching templates for HImode that can never be matched by the hardware.
The HX iterator ([(HI "!TARGET_64BIT") (SI "TARGET_64BIT")]) perfectly aligns 
with the hardware semantics. It ensures that:
On RV32, we only match HImode inputs.
On RV64, we only match SImode inputs.
This makes the RTL expansion much more efficient and strictly correct.
Regarding the test cases, I noticed that there are already existing test cases 
(zbkb32.c and zbkb64.c) in the testsuite. I have run them locally and verified 
that this change has no negative impact on them. Since the existing tests 
already cover the pack instruction generation and pass successfully, I believe 
they are sufficient to verify the correctness of this patch.
Please let me know if you need any further information. Thanks!



[email protected]
 
From: Jeffrey Law
Date: 2026-06-03 22:03
To: panciyan; [email protected]
CC: [email protected]; [email protected]; [email protected]; 
[email protected]; [email protected]; [email protected]; 
[email protected]
Subject: Re: [PATCH] Fix data type iterator of pack rs reg
 
 
On 6/1/2026 2:24 AM, panciyan wrote:
> This patch would like to Fix data type iterator of pack rs reg
>
> Signed-off-by: Ciyan Pan <[email protected]>
> gcc/ChangeLog:
>
>          * config/riscv/crypto.md:
>
> ---
You need to indicate what you changed in you ChangeLog entry.  Perhaps 
something like
 
    * config/riscv/crypto.md (riscv_pack_<X:mode><HX:mode>): Use HX to 
enforce source
    operands are half words rather than HISI would would allow HI 
sources with DI destination.
 
 
As Pan mentioned, it's advisable to include a testcase. While it's 
unlikely we'd regress this code in the future, a regression testcase 
provides  good safety net.
 
Depending on the failure mode you might want a simple compilation test 
or an end-to-end execution test.  The latter are a bit more complex as 
you have to ensure the test is only run on designs with the appropriate 
extension (zbkb in this case).   The basic structure would look like:
 
 
> /* { dg-do run { target riscv_zbkb } } */
> /* { dg-options "-march=rv64gc_zbkb" { target { rv64 } } } */
> /* { dg-options "-march=rv32gc_zbkb" { target { rv32 } } } */
>
>
> /* TESTCASE source */
>
>
 
To run your test (and not the rest of the testsuite)
 
make check-gcc RUNTESTFLAGS=riscv.exp=<your test name>.c
 
Jeff

Reply via email to