On Thu, Dec 7, 2023 at 11:18 AM Liao Shihua <shi...@iscas.ac.cn> wrote:
>
> In accordance with the suggestions of Christoph Müllner, the following 
> amendments are made
>
> Update v1 -> v2:
>   1. Rename *_intrinsic-* to *_intrinsic-XLEN.
>   2. Typo fix.
>   3. Intrinsics with immediate arguments will use marcos at O0 .
>
> It's a little patch add just provides a mapping from the RV intrinsics to the 
> builtin
> names within GCC.

Thanks for the update!

I think this patchset was not properly tested as I see the tests failing.

$ /opt/riscv-mainline/bin/riscv64-unknown-linux-gnu-gcc
-march=rv64gc_zbb_zbc_zbkb_zbkc_zbkx -mabi=lp64d
/home/cm/src/gcc/riscv-mainline/gcc/testsuite/gcc.target/riscv/scalar_bitmanip_intrinsic-64.c
In file included from
/home/cm/src/gcc/riscv-mainline/gcc/testsuite/gcc.target/riscv/scalar_bitmanip_intrinsic-64.c:5:
/opt/riscv-mainline/lib/gcc/riscv64-unknown-linux-gnu/14.0.0/include/riscv_bitmanip.h:
In function '__riscv_orc_b_32':
/opt/riscv-mainline/lib/gcc/riscv64-unknown-linux-gnu/14.0.0/include/riscv_bitmanip.h:61:10:
error: implicit declaration of function '__builtin_riscv_orc_b_32';
did you mean '__builtin_riscv_orc_b_64'?
[-Wimplicit-function-declaration]
   61 |   return __builtin_riscv_orc_b_32 (x);
      |          ^~~~~~~~~~~~~~~~~~~~~~~~
      |          __builtin_riscv_orc_b_64

The spec says: Emulated with rev8+sext.w on RV64.
But I think this is a bug in the spec and should be "orc.b + sext.w".
Still, you need to handle that somehow.

$ /opt/riscv-mainline/bin/riscv64-unknown-linux-gnu-gcc
-march=rv64gc_zknd_zkne_zknh_zksed_zksh -mabi=lp64 -mabi=lp64d
/home/cm/src/gcc/riscv-mainline/gcc/testsuite/gcc.target/riscv/scalar_crypto_intrinsic-64.c
/tmp/ccynQLn2.s: Assembler messages:
/tmp/ccynQLn2.s:127: Error: instruction aes64ks1i requires absolute expression
/tmp/ccynQLn2.s:593: Error: instruction sm4ed requires absolute expression
/tmp/ccynQLn2.s:633: Error: instruction sm4ks requires absolute expression

The absolute expression means that you cannot use a variable but must
use an immediate.
E.g.:
uint64_t foo4(uint64_t rs1)
{
    return __riscv_aes64ks1i(rs1, 3);
}
Here the 3 will be encoded into the instruction.

There are probably more issues, but I stopped investigating after these two.

Also, there are some missing spaces to separate arguments. E.g.:
  return __riscv_aes64ks1i(rs1,rnum);
...should be...
  return __riscv_aes64ks1i(rs1, rnum);

Please make sure to test these patches for RV32 and RV64 before
sending a new revision.
If you run into issues that you can't resolve, then just reach out.

BR
Christoph

>
>
> Liao Shihua (2):
>   Add C intrinsics of Scalar Crypto Extension
>   Add C intrinsics of Bitmanip Extension
>
>  gcc/config.gcc                                |   2 +-
>  gcc/config/riscv/riscv-builtins.cc            |  22 ++
>  gcc/config/riscv/riscv-ftypes.def             |   2 +
>  gcc/config/riscv/riscv-scalar-crypto.def      |  18 +
>  gcc/config/riscv/riscv_bitmanip.h             | 297 +++++++++++++++++
>  gcc/config/riscv/riscv_crypto.h               | 309 ++++++++++++++++++
>  .../riscv/scalar_bitmanip_intrinsic-32.c      |  97 ++++++
>  .../riscv/scalar_bitmanip_intrinsic-64.c      | 115 +++++++
>  .../riscv/scalar_crypto_intrinsic-32.c        | 115 +++++++
>  .../riscv/scalar_crypto_intrinsic-64.c        | 122 +++++++
>  10 files changed, 1098 insertions(+), 1 deletion(-)
>  create mode 100644 gcc/config/riscv/riscv_bitmanip.h
>  create mode 100644 gcc/config/riscv/riscv_crypto.h
>  create mode 100644 
> gcc/testsuite/gcc.target/riscv/scalar_bitmanip_intrinsic-32.c
>  create mode 100644 
> gcc/testsuite/gcc.target/riscv/scalar_bitmanip_intrinsic-64.c
>  create mode 100644 
> gcc/testsuite/gcc.target/riscv/scalar_crypto_intrinsic-32.c
>  create mode 100644 
> gcc/testsuite/gcc.target/riscv/scalar_crypto_intrinsic-64.c
>
> --
> 2.34.1
>

Reply via email to