[llvm] [clang] Remove experimental from Vector Crypto extensions (PR #69000)
https://github.com/4vtomat closed https://github.com/llvm/llvm-project/pull/69000 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[llvm] [clang] Remove experimental from Vector Crypto extensions (PR #69000)
4vtomat wrote: moved to the new [one](https://github.com/llvm/llvm-project/pull/74213) https://github.com/llvm/llvm-project/pull/69000 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[llvm] [clang] Remove experimental from Vector Crypto extensions (PR #69000)
ebiggers wrote: No activity here in a few weeks, so I've opened https://github.com/llvm/llvm-project/pull/74213 with an updated version of the change. https://github.com/llvm/llvm-project/pull/69000 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[llvm] [clang] Remove experimental from Vector Crypto extensions (PR #69000)
@@ -141,6 +141,23 @@ on support follow. ``Zve64f`` Supported ``Zve64d`` Supported ``Zvfh`` Supported + ``Zvbb`` Supported + ``Zvbc`` Supported + ``Zvkb`` Supported + ``Zvkg`` Supported + ``Zvkn`` Supported + ``Zvkn`` Supported ebiggers wrote: Zvkn is in this list twice https://github.com/llvm/llvm-project/pull/69000 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[llvm] [clang] Remove experimental from Vector Crypto extensions (PR #69000)
ebiggers wrote: FYI, I tried squashing the commits of this pull request and rebasing onto `main`, but there are conflicts in several files https://github.com/llvm/llvm-project/pull/69000 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[llvm] [clang] Remove experimental from Vector Crypto extensions (PR #69000)
ebiggers wrote: Hi! What's the status of this pull request? Are there any major issues remaining? It would be very useful to have the LLVM assembler officially support the vector crypto extensions. I've been reviewing the [RISC-V vector crypto accelerated crypto routines for the Linux kernel](https://lore.kernel.org/linux-crypto/20231025183644.8735-1-jerry.s...@sifive.com), and currently they're emitting the instructions as bare `.inst`s in order to not rely on the assembler, which is quite ugly. The GNU assembler already released the support for this a few months ago, in v2.41. FYI @JerryShih and @phoebesv https://github.com/llvm/llvm-project/pull/69000 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[llvm] [clang] Remove experimental from Vector Crypto extensions (PR #69000)
https://github.com/topperc commented: Do we need to add the "experimental" feature to RISCVFeatures.td? If the feature string shows up in the function attributes, won't the backend print that it doesn't recognize the feature name when it parses the string? https://github.com/llvm/llvm-project/pull/69000 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[llvm] [clang] Remove experimental from Vector Crypto extensions (PR #69000)
@@ -106,6 +106,8 @@ static const RISCVSupportedExtension SupportedExtensions[]
= {
{"zdinx", RISCVExtensionVersion{1, 0}},
+{"zexperimental", RISCVExtensionVersion{1, 0}},
+
4vtomat wrote:
No, we don't~
https://github.com/llvm/llvm-project/pull/69000
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[llvm] [clang] Remove experimental from Vector Crypto extensions (PR #69000)
4vtomat wrote: > > Since it's possible that **ISA spec** and **intrinsics spec** are not > > synchronized, so the updates add an dummy extension called > > **zexperimental**, once `-menable-experimental-extensions` is specified, > > the feature `zexperimental` is automatically added. > > If `let RequiredFeatures = ["Zexperimental"] ` is added in `.td` file, it > > will check if `zexperimental` exists, if not, the corresponding intrinsics > > would not be added, hence causing an **intrinsics undeclared** error. > > Thanks for looking into this. It's perhaps surprising we've gone so long > without having the infrastructure to make an extension non-experimental > without making its not-yet-finalized intrinsics non-experimental. Introducing > a new required feature seems like a reasonable way of doing this, but > introducing a dummy extension seems unnecessary (unless I'm missing > something?). > > I could see a solution involving a Clang flag (e.g. > `-menable-experimental-intrinsics`), or perhaps a solution where the user > must use a certain `#define` (I know riscv_vector.h is implemented using a > pragma, but presumably that could still query defines). There may be other > options too. > > Another piece of info that would be helpful to better understand is what the > process is for finalising intrinsic additions (@eopXD might know?). If it's > just the case that we have to wait another month before moving an extension > from experimental to experimental then maybe extra infro to separate > experimental intrinsics from instructions is less interesting. If it's more > indeterminate than that, we probably need it. > > I'll put this on the agenda for Thursday's RISC-V LLVM sync-up call in the > hope we can resolve it there. Yeah, we don't need a new extension actually, thanks for pointing out! https://github.com/llvm/llvm-project/pull/69000 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[llvm] [clang] Remove experimental from Vector Crypto extensions (PR #69000)
asb wrote: > Since it's possible that **ISA spec** and **intrinsics spec** are not > synchronized, so the updates add an dummy extension called **zexperimental**, > once `-menable-experimental-extensions` is specified, the feature > `zexperimental` is automatically added. > If `let RequiredFeatures = ["Zexperimental"] ` is added in `.td` file, it > will check if `zexperimental` exists, if not, the corresponding intrinsics > would not be added, hence causing an **intrinsics undeclared** error. Thanks for looking into this. It's perhaps surprising we've gone so long without having the infrastructure to make an extension non-experimental without making its not-yet-finalized intrinsics non-experimental. Introducing a new required feature seems like a reasonable way of doing this, but introducing a dummy extension seems unnecessary (unless I'm missing something?). I could see a solution involving a Clang flag (e.g. `-menable-experimental-intrinsics`), or perhaps a solution where the user must use a certain `#define` (I know riscv_vector.h is implemented using a pragma, but presumably that could still query defines). There may be other options too. Another piece of info that would be helpful to better understand is what the process is for finalising intrinsic additions (@eopxd might know?). If it's just the case that we have to wait another month before moving an extension from experimental to experimental then maybe extra infro to separate experimental intrinsics from instructions is less interesting. If it's more indeterminate than that, we probably need it. I'll put this on the agenda for Thursday's RISC-V LLVM sync-up call in the hope we can resolve it there. https://github.com/llvm/llvm-project/pull/69000 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] Remove experimental from Vector Crypto extensions (PR #69000)
4vtomat wrote: > Thanks for the patch, some very quick feedback and I'd highlight the first > bullet as the most important, as this is potentially a blocker for graduating > these extensions from experimental. > > * My big concern with this would be the intrinsics - could you please comment > on the status of their standardisation? > * While doing this change, it would make sense to update the header of > RISCVInstrInfoZvk.td. While it doesn't explicitly say it describes an > experimental version of the extension, it references 1.0.0-rc1 of the spec > while presumably there's now a 1.0.0-final? > * Please update llvm/docs/RISCVUsage.rst > * Please add a release note to llvm/docs/ReleaseNotes.rst Currently I guess we're not finalized yet in https://github.com/riscv-non-isa/rvv-intrinsic-doc/pull/234. > > Edit: When committing after review, the commit should be titled something > like "[RISCV] Remove experimental from vector crypto extensions". Oh, thanks, I forgot it~ https://github.com/llvm/llvm-project/pull/69000 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] Remove experimental from Vector Crypto extensions (PR #69000)
github-actions[bot] wrote: :warning: C/C++ code formatter, clang-format found issues in your code. :warning: You can test this locally with the following command: ``bash git-clang-format --diff ba79fb2e1ff7130cde02fbbd325f0f96f8a522ca 521060552794304a5f6b31e38c25ab0a7a0353ff -- clang/lib/Sema/SemaChecking.cpp clang/lib/Sema/SemaRISCVVectorLookup.cpp clang/test/CodeGen/RISCV/rvv-intrinsics-autogenerated/non-policy/non-overloaded/vaesdf.c clang/test/CodeGen/RISCV/rvv-intrinsics-autogenerated/non-policy/non-overloaded/vaesdm.c clang/test/CodeGen/RISCV/rvv-intrinsics-autogenerated/non-policy/non-overloaded/vaesef.c clang/test/CodeGen/RISCV/rvv-intrinsics-autogenerated/non-policy/non-overloaded/vaesem.c clang/test/CodeGen/RISCV/rvv-intrinsics-autogenerated/non-policy/non-overloaded/vaeskf1.c clang/test/CodeGen/RISCV/rvv-intrinsics-autogenerated/non-policy/non-overloaded/vaeskf2.c clang/test/CodeGen/RISCV/rvv-intrinsics-autogenerated/non-policy/non-overloaded/vaesz.c clang/test/CodeGen/RISCV/rvv-intrinsics-autogenerated/non-policy/non-overloaded/vandn.c clang/test/CodeGen/RISCV/rvv-intrinsics-autogenerated/non-policy/non-overloaded/vbrev.c clang/test/CodeGen/RISCV/rvv-intrinsics-autogenerated/non-policy/non-overloaded/vbrev8.c clang/test/CodeGen/RISCV/rvv-intrinsics-autogenerated/non-policy/non-overloaded/vclmul.c clang/test/CodeGen/RISCV/rvv-intrinsics-autogenerated/non-policy/non-overloaded/vclmulh.c clang/test/CodeGen/RISCV/rvv-intrinsics-autogenerated/non-policy/non-overloaded/vclz.c clang/test/CodeGen/RISCV/rvv-intrinsics-autogenerated/non-policy/non-overloaded/vcpopv.c clang/test/CodeGen/RISCV/rvv-intrinsics-autogenerated/non-policy/non-overloaded/vctz.c clang/test/CodeGen/RISCV/rvv-intrinsics-autogenerated/non-policy/non-overloaded/vghsh.c clang/test/CodeGen/RISCV/rvv-intrinsics-autogenerated/non-policy/non-overloaded/vgmul.c clang/test/CodeGen/RISCV/rvv-intrinsics-autogenerated/non-policy/non-overloaded/vrev8.c clang/test/CodeGen/RISCV/rvv-intrinsics-autogenerated/non-policy/non-overloaded/vrol.c clang/test/CodeGen/RISCV/rvv-intrinsics-autogenerated/non-policy/non-overloaded/vror.c clang/test/CodeGen/RISCV/rvv-intrinsics-autogenerated/non-policy/non-overloaded/vsha2ch.c clang/test/CodeGen/RISCV/rvv-intrinsics-autogenerated/non-policy/non-overloaded/vsha2cl.c clang/test/CodeGen/RISCV/rvv-intrinsics-autogenerated/non-policy/non-overloaded/vsha2ms.c clang/test/CodeGen/RISCV/rvv-intrinsics-autogenerated/non-policy/non-overloaded/vsm3c.c clang/test/CodeGen/RISCV/rvv-intrinsics-autogenerated/non-policy/non-overloaded/vsm3me.c clang/test/CodeGen/RISCV/rvv-intrinsics-autogenerated/non-policy/non-overloaded/vsm4k.c clang/test/CodeGen/RISCV/rvv-intrinsics-autogenerated/non-policy/non-overloaded/vsm4r.c clang/test/CodeGen/RISCV/rvv-intrinsics-autogenerated/non-policy/non-overloaded/vwsll.c clang/test/CodeGen/RISCV/rvv-intrinsics-autogenerated/non-policy/overloaded/vaesdf.c clang/test/CodeGen/RISCV/rvv-intrinsics-autogenerated/non-policy/overloaded/vaesdm.c clang/test/CodeGen/RISCV/rvv-intrinsics-autogenerated/non-policy/overloaded/vaesef.c clang/test/CodeGen/RISCV/rvv-intrinsics-autogenerated/non-policy/overloaded/vaesem.c clang/test/CodeGen/RISCV/rvv-intrinsics-autogenerated/non-policy/overloaded/vaeskf1.c clang/test/CodeGen/RISCV/rvv-intrinsics-autogenerated/non-policy/overloaded/vaeskf2.c clang/test/CodeGen/RISCV/rvv-intrinsics-autogenerated/non-policy/overloaded/vaesz.c clang/test/CodeGen/RISCV/rvv-intrinsics-autogenerated/non-policy/overloaded/vandn.c clang/test/CodeGen/RISCV/rvv-intrinsics-autogenerated/non-policy/overloaded/vbrev.c clang/test/CodeGen/RISCV/rvv-intrinsics-autogenerated/non-policy/overloaded/vbrev8.c clang/test/CodeGen/RISCV/rvv-intrinsics-autogenerated/non-policy/overloaded/vclmul.c clang/test/CodeGen/RISCV/rvv-intrinsics-autogenerated/non-policy/overloaded/vclmulh.c clang/test/CodeGen/RISCV/rvv-intrinsics-autogenerated/non-policy/overloaded/vclz.c clang/test/CodeGen/RISCV/rvv-intrinsics-autogenerated/non-policy/overloaded/vcpopv.c clang/test/CodeGen/RISCV/rvv-intrinsics-autogenerated/non-policy/overloaded/vctz.c clang/test/CodeGen/RISCV/rvv-intrinsics-autogenerated/non-policy/overloaded/vghsh.c clang/test/CodeGen/RISCV/rvv-intrinsics-autogenerated/non-policy/overloaded/vgmul.c clang/test/CodeGen/RISCV/rvv-intrinsics-autogenerated/non-policy/overloaded/vrev8.c clang/test/CodeGen/RISCV/rvv-intrinsics-autogenerated/non-policy/overloaded/vrol.c clang/test/CodeGen/RISCV/rvv-intrinsics-autogenerated/non-policy/overloaded/vror.c clang/test/CodeGen/RISCV/rvv-intrinsics-autogenerated/non-policy/overloaded/vsha2ch.c clang/test/CodeGen/RISCV/rvv-intrinsics-autogenerated/non-policy/overloaded/vsha2cl.c clang/test/CodeGen/RISCV/rvv-intrinsics-autogenerated/non-policy/overloaded/vsha2ms.c clang/test/CodeGen/RISCV/rvv-intrinsics-autogenerated/non-policy/overloaded/vsm3c.c
[clang] Remove experimental from Vector Crypto extensions (PR #69000)
asb wrote: Thanks for the patch, some very quick feedback and I'd highlight the first bullet as the most important, as this is potentially a blocker for graduating these extensions from experimental. * My big concern with this would be the intrinsics - could you please comment on the status of their standardisation? * While doing this change, it would make sense to update the header of RISCVInstrInfoZvk.td. While it doesn't explicitly say it describes an experimental version of the extension, it references 1.0.0-rc1 of the spec while presumably there's now a 1.0.0-final? * Please update llvm/docs/RISCVUsage.rst * Please add a release note to llvm/docs/ReleaseNotes.rst https://github.com/llvm/llvm-project/pull/69000 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
