[Openmp-commits] [PATCH] D132925: Add LoongArch64 Support For OpenMP

[Lu Weining via Phabricator via Openmp-commits]

SixWeining added a comment.

Thanks.

This touches files like D59880 <https://reviews.llvm.org/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 <https://reviews.llvm.org/D109333>.

Some concerns:

1. Have you tested existing testcases on a native LoongArch machine? If so, do you use `clang` or `gcc`?
2. Do we need to add new tests?



================
Comment at: openmp/runtime/CMakeLists.txt:189
     set(RISCV64 TRUE)
+elseif("${LIBOMP_ARCH}" STREQUAL "loongarch64") # LOONGARCH64 architecture
+    set(LOONGARCH64 TRUE)
----------------
nit: Maybe `LoongArch64` is better.


================
Comment at: openmp/runtime/src/z_Linux_asm.S:1762-1770
+// Te  if(b==0)
+//
+//       return a;
+//
+//  return gcd(b,a%b);
+//
+//}
----------------
What does that mean?


================
Comment at: openmp/runtime/src/z_Linux_asm.S:1824
+	// Align the stack to 16 bytes
+
+	addi.d $t8, $zero, -16
----------------
Nit: useless blank line


================
Comment at: openmp/runtime/src/z_Linux_asm.S:1833
+#if OMPT_SUPPORT
+	// Save frame pointer into exit_frame
+	st.d	$fp, $a5, 0
----------------
Nit: generally speaking, comment sentence should ends with a dot('.').


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D132925/new/

https://reviews.llvm.org/D132925