Re: [PATCH 2/2] RISC-V: Make SHA-256, SM3 and SM4 builtins operate on uint32_t
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
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
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