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

[WÁNG Xuěruì via Phabricator via Openmp-commits]

xen0n added a comment.

Thanks and welcome to the LLVM community! Some minor comments regarding LoongArch and general coding style.



================
Comment at: openmp/README.rst:140
 
-**LIBOMP_ARCH** = ``aarch64|arm|i386|mic|mips|mips64|ppc64|ppc64le|x86_64|riscv64``
+**LIBOMP_ARCH** = ``aarch64|arm|i386|mic|mips|mips64|ppc64|ppc64le|x86_64|riscv64|loongarch64``
   The default value for this option is chosen based on probing the compiler for
----------------
The list is alphabetically sorted, so please put this before "mic".


================
Comment at: openmp/runtime/src/kmp_csupport.cpp:703
 #elif (KMP_ARCH_ARM || KMP_ARCH_AARCH64 || KMP_ARCH_MIPS || KMP_ARCH_MIPS64 || \
-       KMP_ARCH_RISCV64)
+       KMP_ARCH_RISCV64 || KMP_ARCH_LOONGARCH64)
 // Nothing to see here move along
----------------
Similarly place this before MIPS.


================
Comment at: openmp/runtime/src/z_Linux_asm.S:1777
+	.globl	__kmp_invoke_microtask
+	.p2align	1
+	.type	__kmp_invoke_microtask, at function
----------------
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.


================
Comment at: openmp/runtime/src/z_Linux_asm.S:1817-1818
+
+	addi.d $t8, $zero, -16
+	and	$sp, $sp, $t8
+
----------------
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]`".)


================
Comment at: openmp/runtime/src/z_Linux_asm.S:1762-1770
+// Te  if(b==0)
+//
+//       return a;
+//
+//  return gcd(b,a%b);
+//
+//}
----------------
SixWeining wrote:
> What does that mean?
I guess it means the return value is in a0 which is always 1.


================
Comment at: openmp/runtime/tools/lib/Platform.pm:102
         "riscv64" => "RISC-V (64-bit)",
+        "loongarch64" => "LoongArch64",
     );
----------------
Place before "mic".


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132925