cpu: add loongarch64 support #1923
Conversation
| @@ -74,10 +74,10 @@ oneDNN supports platforms based on the following architectures: | |||
| - [OpenPOWER](https://openpowerfoundation.org/) / [IBM Power ISA](https://en.wikipedia.org/wiki/Power_ISA). | |||
| - [IBMz z/Architecture (s390x)](https://en.wikipedia.org/wiki/Z/Architecture). | |||
| - [RISC-V 64-bit (RV64)](https://en.wikipedia.org/wiki/RISC-V). | |||
|
|
|||
| - [LOONGARCH 64 bit](https://docs.kernel.org/arch/loongarch/introduction.html). | |||
There was a problem hiding this comment.
Please restore the new line to separate the warning.
src/cpu/platform.hpp
Outdated
| // - DNNL_ARCH_GENERIC | ||
| // Target architecture macro is set to 1, others to 0. All macros are defined. | ||
|
|
||
| #if defined(DNNL_X64) + defined(DNNL_AARCH64) + defined(DNNL_PPC64) \ | ||
| + defined(DNNL_S390X) + defined(DNNL_RV64) \ | ||
| + defined(DNNL_S390X) + defined(DNNL_RV64) + defined(DNNL_LOONGARCH64) \ |
There was a problem hiding this comment.
Please apply clang-format on this file.
There was a problem hiding this comment.
Please apply clang-format on this file.
It's been revised and resubmitted, thanks
There was a problem hiding this comment.
thanks for the PR. I have few questions:
- could you reword the commit message to align with oneDNN guidelines (see here. Something like
cpu: add loongarch64 supportshould do. - In general, adding a new CPU to oneDNN platform.cmake is necessary only when isa specific optimization or fixes are added to the library. From the look of the patch, it does not seem to be the case for loongarch64 (this patch is equivalent to building the library with
ONEDNN_ARCH_OPT_FLAGS=-march=loongarch64cmake option). Do you have any plan to add loongarch64 isa specific code to oneDNN?
| @@ -363,6 +363,8 @@ elseif(UNIX OR MINGW) | |||
| elseif(DNNL_TARGET_ARCH STREQUAL "RV64") | |||
| # G = General-purpose extensions, C = Compression extension (very common). | |||
| append(DEF_ARCH_OPT_FLAGS "-march=rv64gc") | |||
| elseif(DNNL_TARGET_ARCH STREQUAL "LOONGARCH64") | |||
| append(DEF_ARCH_OPT_FLAGS "-march=loongarch64") | |||
There was a problem hiding this comment.
Any reason to not add opt flags for compilers other than gcc ?
loong-hy
changed the title
Hello, I have changed the commit as requested.
Because I will add loongarch related code later, I didn't change it this time because of time constraints. |
loong-hy
changed the title
|
@loong-hy do you mind submitting this patch as part of the future PR that will add loongarch specific code? |
Sure, I'll revise and resubmit the code shortly, thanks! |
about loongarch：https://docs.kernel.org/arch/loongarch/introduction.html