LoongArch64 is a new risc architecture established by Loongson. There has been PC which contains 3A5000 using this architecture. Now gcc, glibc and binutils have added support for it, while linux mainline will fully support it after 6.1.
This patch adds support for LLVM OpenMP following D59880 for RISCV64.
Details
Diff Detail
- Repository
- rG LLVM Github Monorepo
Event Timeline
Thanks.
This touches files like D59880 did except openmp/runtime/src/thirdparty/ittnotify/ittnotify_config.h and outdated openmp/www. For ittnotify I suppose we do not need to change it because D109333.
Some concerns:
- Have you tested existing testcases on a native LoongArch machine? If so, do you use clang or gcc?
- Do we need to add new tests?
openmp/runtime/CMakeLists.txt | ||
---|---|---|
189 | nit: Maybe LoongArch64 is better. | |
openmp/runtime/src/z_Linux_asm.S | ||
1762–1770 | What does that mean? | |
1824 | Nit: useless blank line | |
1833 | Nit: generally speaking, comment sentence should ends with a dot('.'). |
For 1, I tried to use "make check", unfortunately, almost all test unit failed. Threads crash just before ending and it seems to be about stack. The crash happened at the end of process kmp_invoke_task_func near kmp_run_after_invoked_task. I 'm trying to find the possible cause.
For 2, because RISC-V doesn't have particular test, and I think common tests on loongarch are adequate to find errors, it is not neccessary to add new tests.
Now I have fixed the problem(The cause is just about not saving return address before calling). Although that, it still failed 4 tests after I ran "make check-openmp". The failed tests are affinity/kmp-affinity.c, affinity/kmp-hw-subset.c, affinity/omp-places.c and teams/teams.c. I 'm not sure if it's necessary to care about and how to handle them. There are also 1 unexpextedly passed and 3 expectedly failed tests.
The comment in CMakeList.txt has been fixed. The 1762-1770 lines in z_Linux_asm.S are just rubbish code I carelessly pasted.
For the dot after comment, in z_Linux_asm.S, it is the same as riscv. Is it also necessary to modify them?
Excellent! I haven’t saw this ra issue when I review the patch.
I think you’re using gcc to build the openmp since clang currently is unable to build such complex programs, right?
AFAIK, some tests can only be run by clang. check-openmp is only part of all tests. There are some other targets like check-libomptarget IIRC. So I think it’s better to mention your testing environment, methods and results in the patch summary.
And I’m not a regular reviewer of the openmp subproject. Let’s wait others’ inputs.
Thanks and welcome to the LLVM community! Some minor comments regarding LoongArch and general coding style.
openmp/README.rst | ||
---|---|---|
140 | The list is alphabetically sorted, so please put this before "mic". | |
openmp/runtime/src/kmp_csupport.cpp | ||
703 | Similarly place this before MIPS. | |
openmp/runtime/src/z_Linux_asm.S | ||
1762–1770 | I guess it means the return value is in a0 which is always 1. | |
1777 | RISCV instruction words can be 2 bytes long due to RVC, but LoongArch insns are always 4 bytes long. So the p2align value probably should be 2. | |
1817–1818 | LoongArch base ISA has fairly decent bitops, so we can simply bstrins.d $sp, $zero, 3, 0 to clear the lowest 4 bits. (Literally "insert $zero's content into $sp[3:0]".) | |
openmp/runtime/tools/lib/Platform.pm | ||
102 | Place before "mic". |
openmp/runtime/src/z_Linux_asm.S | ||
---|---|---|
1802–1808 | Let's be less tricky and reduce one instruction. |
openmp/runtime/src/z_Linux_asm.S | ||
---|---|---|
1806 | Hmm, looks like something has gone wrong here when my change is applied... addi.d $t0, $t0, 1 is duplicated, and addi.d $t0, $a3, -6 is missing. |
openmp/runtime/src/z_Linux_asm.S | ||
---|---|---|
1805 | Should place this line before line 1802? |
This change LGTM regarding LoongArch.
a samll tip: The patch's summary can be edited in this web page in case you want to mention the testing result. :)
This works on a GCC compiler farm machine.
gcc -fopenmp a.c -g -o a -I ~/llvm/out/release/projects/openmp/runtime/src ~/llvm/out/release/lib/libomp.so -Wl,-rpath=$HOME/llvm/out/release/lib ./a # good
The list is alphabetically sorted, so please put this before "mic".