cpu: add loongarch64 support #1923
Open
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
dzarukin
reviewed
May 20, 2024
| @@ -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
mgouicem
reviewed
May 22, 2024
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
about loongarch：https://docs.kernel.org/arch/loongarch/introduction.html