Re: [PATCH 2/2] RISC-V: Make SHA-256, SM3 and SM4 builtins operate on uint32_t

2023-09-11 Thread Tsukasa OI via Gcc-patches
On 2023/09/12 11:44, Kito Cheng wrote:
> LGTM, I think llvm and GCC are inconsistent for those intrinsics API
> is really unfortunate...so really appreciate making those API align :)

I guess that you mean LGTM to this patch set (given the context).
If so, I will commit this patch set later.

> And did you have plan to add riscv_crypto.h after updating/fixing all builtin?

Like riscv_vector.h?  I don't and I think we need to discuss what to
include in... like Toolchain SIG first.

Additionally, we need to discuss what to do with these XLEN-specific
builtins specific to LLVM (my idea is, add XLEN-specific builtins like
LLVM but not to remove XLEN independent ones).

Thanks,
Tsukasa

> Excerpt from the latest clang/include/clang/Basic/BuiltinsRISCV.def:
> 
> // Zbb extension
> TARGET_BUILTIN(__builtin_riscv_orc_b_32, "UiUi", "nc", "zbb")
> TARGET_BUILTIN(__builtin_riscv_orc_b_64, "UWiUWi", "nc", "zbb,64bit")
> TARGET_BUILTIN(__builtin_riscv_clz_32, "UiUi", "nc", "zbb|xtheadbb")
> TARGET_BUILTIN(__builtin_riscv_clz_64, "UiUWi", "nc", "zbb|xtheadbb,64bit")
> TARGET_BUILTIN(__builtin_riscv_ctz_32, "UiUi", "nc", "zbb")
> TARGET_BUILTIN(__builtin_riscv_ctz_64, "UiUWi", "nc", "zbb,64bit")
> 
> // Zbc or Zbkc extension
> TARGET_BUILTIN(__builtin_riscv_clmul_32, "UiUiUi", "nc", "zbc|zbkc")
> TARGET_BUILTIN(__builtin_riscv_clmul_64, "UWiUWiUWi", "nc", "zbc|zbkc,64bit")
> TARGET_BUILTIN(__builtin_riscv_clmulh_32, "UiUiUi", "nc", "zbc|zbkc,32bit")
> TARGET_BUILTIN(__builtin_riscv_clmulh_64, "UWiUWiUWi", "nc", "zbc|zbkc,64bit")
> TARGET_BUILTIN(__builtin_riscv_clmulr_32, "UiUiUi", "nc", "zbc,32bit")
> TARGET_BUILTIN(__builtin_riscv_clmulr_64, "UWiUWiUWi", "nc", "zbc,64bit")
> 
> // Zbkx
> TARGET_BUILTIN(__builtin_riscv_xperm4_32, "UiUiUi", "nc", "zbkx,32bit")
> TARGET_BUILTIN(__builtin_riscv_xperm4_64, "UWiUWiUWi", "nc", "zbkx,64bit")
> TARGET_BUILTIN(__builtin_riscv_xperm8_32, "UiUiUi", "nc", "zbkx,32bit")
> TARGET_BUILTIN(__builtin_riscv_xperm8_64, "UWiUWiUWi", "nc", "zbkx,64bit")
> 
> // Zbkb extension
> TARGET_BUILTIN(__builtin_riscv_brev8_32, "UiUi", "nc", "zbkb")
> TARGET_BUILTIN(__builtin_riscv_brev8_64, "UWiUWi", "nc", "zbkb,64bit")
> TARGET_BUILTIN(__builtin_riscv_zip_32, "UiUi", "nc", "zbkb,32bit")
> TARGET_BUILTIN(__builtin_riscv_unzip_32, "UiUi", "nc", "zbkb,32bit")


Re: [PATCH 2/2] RISC-V: Make SHA-256, SM3 and SM4 builtins operate on uint32_t

2023-09-11 Thread Kito Cheng via Gcc-patches
LGTM, I think llvm and GCC are inconsistent for those intrinsics API
is really unfortunate...so really appreciate making those API align :)
And did you have plan to add riscv_crypto.h after updating/fixing all builtin?

On Tue, Sep 12, 2023 at 9:29 AM Tsukasa OI via Gcc-patches
 wrote:
>
> From: Tsukasa OI 
>
> This is in parity with the LLVM commit a64b3e92c7cb ("[RISCV] Re-define
> sha256, Zksed, and Zksh intrinsics to use i32 types.").
>
> SHA-256, SM3 and SM4 instructions operate on 32-bit integers and upper
> 32-bits have no effects on RV64 (the output is sign-extended from the
> original 32-bit value).  In that sense, making those intrinsics only
> operate on uint32_t is much more natural than XLEN-bits wide integers.
>
> This commit reforms instructions and expansions based on 32-bit
> instruction handling on RV64 (such as ADDW).
>
> Before:
>riscv__si: For RV32, fully operate on uint32_t
>riscv__di: For RV64, fully operate on uint64_t
> After:
>   *riscv__si: For RV32, fully operate on uint32_t
>riscv__di_extended:
>   For RV64.  Input is uint32_t and output is int64_t,
>   sign-extended from the int32_t result
>   (represents a part of  behavior).
>riscv__si: Common (fully operate on uint32_t).
>   On RV32, "expands" to *riscv__si.
>   On RV64, initially expands to riscv__di_extended *and*
>   extracts lower 32-bits from the int64_t result.
>
> It also refines definitions of SHA-256, SM3 and SM4 intrinsics.
>
> gcc/ChangeLog:
>
> * config/riscv/crypto.md (riscv_sha256sig0_,
> riscv_sha256sig1_, riscv_sha256sum0_,
> riscv_sha256sum1_, riscv_sm3p0_, riscv_sm3p1_,
> riscv_sm4ed_, riscv_sm4ks_): Remove and replace with
> new insn/expansions.
> (SHA256_OP, SM3_OP, SM4_OP): New iterators.
> (sha256_op, sm3_op, sm4_op): New attributes for iteration.
> (*riscv__si): New raw instruction for RV32.
> (*riscv__si): Ditto.
> (*riscv__si): Ditto.
> (riscv__di_extended): New base instruction for RV64.
> (riscv__di_extended): Ditto.
> (riscv__di_extended): Ditto.
> (riscv__si): New common instruction expansion.
> (riscv__si): Ditto.
> (riscv__si): Ditto.
> * config/riscv/riscv-builtins.cc: Add availability "crypto_zknh",
> "crypto_zksh" and "crypto_zksed".  Remove availability
> "crypto_zksh{32,64}" and "crypto_zksed{32,64}".
> * config/riscv/riscv-ftypes.def: Remove unused function type.
> * config/riscv/riscv-scalar-crypto.def: Make SHA-256, SM3 and SM4
> intrinsics to operate on uint32_t.
>
> gcc/testsuite/ChangeLog:
>
> * gcc.target/riscv/zknh-sha256.c: Moved to...
> * gcc.target/riscv/zknh-sha256-64.c: ...here.  Test RV64.
> * gcc.target/riscv/zknh-sha256-32.c: New test for RV32.
> * gcc.target/riscv/zksh64.c: Change the type.
> * gcc.target/riscv/zksed64.c: Ditto.
> ---
>  gcc/config/riscv/crypto.md| 161 --
>  gcc/config/riscv/riscv-builtins.cc|   7 +-
>  gcc/config/riscv/riscv-ftypes.def |   1 -
>  gcc/config/riscv/riscv-scalar-crypto.def  |  24 +--
>  .../gcc.target/riscv/zknh-sha256-32.c |  10 ++
>  .../riscv/{zknh-sha256.c => zknh-sha256-64.c} |   8 +-
>  gcc/testsuite/gcc.target/riscv/zksed64.c  |   4 +-
>  gcc/testsuite/gcc.target/riscv/zksh64.c   |   4 +-
>  8 files changed, 139 insertions(+), 80 deletions(-)
>  create mode 100644 gcc/testsuite/gcc.target/riscv/zknh-sha256-32.c
>  rename gcc/testsuite/gcc.target/riscv/{zknh-sha256.c => zknh-sha256-64.c} 
> (78%)
>
> diff --git a/gcc/config/riscv/crypto.md b/gcc/config/riscv/crypto.md
> index e4b7f0190dfe..03a1d03397d9 100644
> --- a/gcc/config/riscv/crypto.md
> +++ b/gcc/config/riscv/crypto.md
> @@ -250,36 +250,47 @@
>
>  ;; ZKNH - SHA256
>
> -(define_insn "riscv_sha256sig0_"
> -  [(set (match_operand:X 0 "register_operand" "=r")
> -(unspec:X [(match_operand:X 1 "register_operand" "r")]
> -  UNSPEC_SHA_256_SIG0))]
> -  "TARGET_ZKNH"
> -  "sha256sig0\t%0,%1"
> -  [(set_attr "type" "crypto")])
> -
> -(define_insn "riscv_sha256sig1_"
> -  [(set (match_operand:X 0 "register_operand" "=r")
> -(unspec:X [(match_operand:X 1 "register_operand" "r")]
> -  UNSPEC_SHA_256_SIG1))]
> -  "TARGET_ZKNH"
> -  "sha256sig1\t%0,%1"
> +(define_int_iterator SHA256_OP [
> +  UNSPEC_SHA_256_SIG0 UNSPEC_SHA_256_SIG1
> +  UNSPEC_SHA_256_SUM0 UNSPEC_SHA_256_SUM1])
> +(define_int_attr sha256_op [
> +  (UNSPEC_SHA_256_SIG0 "sha256sig0") (UNSPEC_SHA_256_SIG1 "sha256sig1")
> +  (UNSPEC_SHA_256_SUM0 "sha256sum0") (UNSPEC_SHA_256_SUM1 "sha256sum1")])
> +
> +(define_insn "*riscv__si"
> +  [(set (match_operand:SI 0 "register_operand" "=r")
> +(unspec:SI [(match_operand:SI 1 "register_operand" "r")]
> +   SHA256_OP

[PATCH 2/2] RISC-V: Make SHA-256, SM3 and SM4 builtins operate on uint32_t

2023-09-11 Thread Tsukasa OI via Gcc-patches
From: Tsukasa OI 

This is in parity with the LLVM commit a64b3e92c7cb ("[RISCV] Re-define
sha256, Zksed, and Zksh intrinsics to use i32 types.").

SHA-256, SM3 and SM4 instructions operate on 32-bit integers and upper
32-bits have no effects on RV64 (the output is sign-extended from the
original 32-bit value).  In that sense, making those intrinsics only
operate on uint32_t is much more natural than XLEN-bits wide integers.

This commit reforms instructions and expansions based on 32-bit
instruction handling on RV64 (such as ADDW).

Before:
   riscv__si: For RV32, fully operate on uint32_t
   riscv__di: For RV64, fully operate on uint64_t
After:
  *riscv__si: For RV32, fully operate on uint32_t
   riscv__di_extended:
  For RV64.  Input is uint32_t and output is int64_t,
  sign-extended from the int32_t result
  (represents a part of  behavior).
   riscv__si: Common (fully operate on uint32_t).
  On RV32, "expands" to *riscv__si.
  On RV64, initially expands to riscv__di_extended *and*
  extracts lower 32-bits from the int64_t result.

It also refines definitions of SHA-256, SM3 and SM4 intrinsics.

gcc/ChangeLog:

* config/riscv/crypto.md (riscv_sha256sig0_,
riscv_sha256sig1_, riscv_sha256sum0_,
riscv_sha256sum1_, riscv_sm3p0_, riscv_sm3p1_,
riscv_sm4ed_, riscv_sm4ks_): Remove and replace with
new insn/expansions.
(SHA256_OP, SM3_OP, SM4_OP): New iterators.
(sha256_op, sm3_op, sm4_op): New attributes for iteration.
(*riscv__si): New raw instruction for RV32.
(*riscv__si): Ditto.
(*riscv__si): Ditto.
(riscv__di_extended): New base instruction for RV64.
(riscv__di_extended): Ditto.
(riscv__di_extended): Ditto.
(riscv__si): New common instruction expansion.
(riscv__si): Ditto.
(riscv__si): Ditto.
* config/riscv/riscv-builtins.cc: Add availability "crypto_zknh",
"crypto_zksh" and "crypto_zksed".  Remove availability
"crypto_zksh{32,64}" and "crypto_zksed{32,64}".
* config/riscv/riscv-ftypes.def: Remove unused function type.
* config/riscv/riscv-scalar-crypto.def: Make SHA-256, SM3 and SM4
intrinsics to operate on uint32_t.

gcc/testsuite/ChangeLog:

* gcc.target/riscv/zknh-sha256.c: Moved to...
* gcc.target/riscv/zknh-sha256-64.c: ...here.  Test RV64.
* gcc.target/riscv/zknh-sha256-32.c: New test for RV32.
* gcc.target/riscv/zksh64.c: Change the type.
* gcc.target/riscv/zksed64.c: Ditto.
---
 gcc/config/riscv/crypto.md| 161 --
 gcc/config/riscv/riscv-builtins.cc|   7 +-
 gcc/config/riscv/riscv-ftypes.def |   1 -
 gcc/config/riscv/riscv-scalar-crypto.def  |  24 +--
 .../gcc.target/riscv/zknh-sha256-32.c |  10 ++
 .../riscv/{zknh-sha256.c => zknh-sha256-64.c} |   8 +-
 gcc/testsuite/gcc.target/riscv/zksed64.c  |   4 +-
 gcc/testsuite/gcc.target/riscv/zksh64.c   |   4 +-
 8 files changed, 139 insertions(+), 80 deletions(-)
 create mode 100644 gcc/testsuite/gcc.target/riscv/zknh-sha256-32.c
 rename gcc/testsuite/gcc.target/riscv/{zknh-sha256.c => zknh-sha256-64.c} (78%)

diff --git a/gcc/config/riscv/crypto.md b/gcc/config/riscv/crypto.md
index e4b7f0190dfe..03a1d03397d9 100644
--- a/gcc/config/riscv/crypto.md
+++ b/gcc/config/riscv/crypto.md
@@ -250,36 +250,47 @@
 
 ;; ZKNH - SHA256
 
-(define_insn "riscv_sha256sig0_"
-  [(set (match_operand:X 0 "register_operand" "=r")
-(unspec:X [(match_operand:X 1 "register_operand" "r")]
-  UNSPEC_SHA_256_SIG0))]
-  "TARGET_ZKNH"
-  "sha256sig0\t%0,%1"
-  [(set_attr "type" "crypto")])
-
-(define_insn "riscv_sha256sig1_"
-  [(set (match_operand:X 0 "register_operand" "=r")
-(unspec:X [(match_operand:X 1 "register_operand" "r")]
-  UNSPEC_SHA_256_SIG1))]
-  "TARGET_ZKNH"
-  "sha256sig1\t%0,%1"
+(define_int_iterator SHA256_OP [
+  UNSPEC_SHA_256_SIG0 UNSPEC_SHA_256_SIG1
+  UNSPEC_SHA_256_SUM0 UNSPEC_SHA_256_SUM1])
+(define_int_attr sha256_op [
+  (UNSPEC_SHA_256_SIG0 "sha256sig0") (UNSPEC_SHA_256_SIG1 "sha256sig1")
+  (UNSPEC_SHA_256_SUM0 "sha256sum0") (UNSPEC_SHA_256_SUM1 "sha256sum1")])
+
+(define_insn "*riscv__si"
+  [(set (match_operand:SI 0 "register_operand" "=r")
+(unspec:SI [(match_operand:SI 1 "register_operand" "r")]
+   SHA256_OP))]
+  "TARGET_ZKNH && !TARGET_64BIT"
+  "\t%0,%1"
   [(set_attr "type" "crypto")])
 
-(define_insn "riscv_sha256sum0_"
-  [(set (match_operand:X 0 "register_operand" "=r")
-(unspec:X [(match_operand:X 1 "register_operand" "r")]
-  UNSPEC_SHA_256_SUM0))]
-  "TARGET_ZKNH"
-  "sha256sum0\t%0,%1"
+(define_insn "riscv__di_extended"
+  [(set (match_operand:DI 0 "register_operand" "=r")
+(sign_extend:DI
+ (unspec:SI [(match_operand:SI 1 "register_ope