Page MenuHomePhabricator

[OpenMP] Added the support for cache line size 256 for A64FX
ClosedPublic

Authored by tianshilei1992 on Dec 12 2020, 1:36 PM.

Details

Summary

Fugaku supercomputer is built with the Fujitsu A64FX microprocessor, whose cache line is 256. In current libomp, we only have cache line size 128 for PPC64 and otherwise 64. This patch added the support of cache line 256 for A64FX. It's worth noting that although A64FX is a variant of AArch64, this property is not shared. As a result, in light of UCX source code (https://github.com/openucx/ucx/blob/392443ab92626412605dee1572056f79c897c6c3/src/ucs/arch/aarch64/cpu.c#L17), we can only determine by checking whether the CPU is FUJITSU A64FX.

Diff Detail

Event Timeline

tianshilei1992 created this revision.Dec 12 2020, 1:36 PM
tianshilei1992 requested review of this revision.Dec 12 2020, 1:36 PM
Herald added a project: Restricted Project. · View Herald Transcript

Fixed a typo in comments

Harbormaster completed remote builds in B82161: Diff 311411.
jdoerfert accepted this revision.Jan 7 2021, 9:01 AM

LGTM, at least I don't see anything obviously problematic

This revision is now accepted and ready to land.Jan 7 2021, 9:01 AM

Why is it necessary to write and compile a C program just to parse /proc/cpuinfo? Can this be done directly from CMake?

Why is it necessary to write and compile a C program just to parse /proc/cpuinfo? Can this be done directly from CMake?

Why is it necessary to write and compile a C program just to parse /proc/cpuinfo? Can this be done directly from CMake?

Unfortunately we can't. CMAKE_SYSTEM_PROCESSOR just reports aarch64.

Updated to make OMPT work

Why is it necessary to write and compile a C program just to parse /proc/cpuinfo? Can this be done directly from CMake?

Unfortunately we can't. CMAKE_SYSTEM_PROCESSOR just reports aarch64.

I understand that the predefined variables don't work, but we can surely parse /proc/cpuinfo with native CMake commands instead of compiling a C program, no?

Parse /proc/cpuinfo with CMake directly

Optimized string match

Removed useless comment

Hahnfeld added inline comments.Jan 7 2021, 11:30 PM
openmp/runtime/cmake/LibompGetArchitecture.cmake
78–81

Can you use TRUE and FALSE here? This also avoids the overly generic MATCHES "1" at call site.

Used TRUE/FALSE instead of 0/1 for LIBOMP_DETECT_AARCH64_A64FX

tianshilei1992 marked an inline comment as done.Jan 9 2021, 7:17 AM
Hahnfeld accepted this revision.Jan 9 2021, 8:55 AM

LGTM, thanks for the changes!

I'm going to revert this as it breaks CMake on systems which do not have /proc/cpuinfo such as macOS.

This may be a bit hard to see because the code isn't reached unless the architecture is aarch64, but on an ARM macOS system that path hits. It would also hit on other BSDs or other OSes running on AArch64 but without /proc/cpuinfo.

For your reference, here is the error message from CMake for me:

CMake Error at /Users/chandlerc/src/llvm/llvm-project/openmp/runtime/cmake/LibompGetArchitecture.cmake:74 (file):
  file failed to open for reading (No such file or directory):

    /proc/cpuinfo
Call Stack (most recent call first):
  /Users/chandlerc/src/llvm/llvm-project/openmp/runtime/CMakeLists.txt:73 (libomp_is_aarch64_a64fx)

I'm going to revert this as it breaks CMake on systems which do not have /proc/cpuinfo such as macOS.

This may be a bit hard to see because the code isn't reached unless the architecture is aarch64, but on an ARM macOS system that path hits. It would also hit on other BSDs or other OSes running on AArch64 but without /proc/cpuinfo.

For your reference, here is the error message from CMake for me:

CMake Error at /Users/chandlerc/src/llvm/llvm-project/openmp/runtime/cmake/LibompGetArchitecture.cmake:74 (file):
  file failed to open for reading (No such file or directory):

    /proc/cpuinfo
Call Stack (most recent call first):
  /Users/chandlerc/src/llvm/llvm-project/openmp/runtime/CMakeLists.txt:73 (libomp_is_aarch64_a64fx)

Will fix it right away.

I'm going to revert this as it breaks CMake on systems which do not have /proc/cpuinfo such as macOS.

This may be a bit hard to see because the code isn't reached unless the architecture is aarch64, but on an ARM macOS system that path hits. It would also hit on other BSDs or other OSes running on AArch64 but without /proc/cpuinfo.

For your reference, here is the error message from CMake for me:

CMake Error at /Users/chandlerc/src/llvm/llvm-project/openmp/runtime/cmake/LibompGetArchitecture.cmake:74 (file):
  file failed to open for reading (No such file or directory):

    /proc/cpuinfo
Call Stack (most recent call first):
  /Users/chandlerc/src/llvm/llvm-project/openmp/runtime/CMakeLists.txt:73 (libomp_is_aarch64_a64fx)

Will fix it right away.

I didn't think my CMake fu was up to it, but I think I have a fix, WDYT: https://reviews.llvm.org/D94889

I'm going to revert this as it breaks CMake on systems which do not have /proc/cpuinfo such as macOS.

This may be a bit hard to see because the code isn't reached unless the architecture is aarch64, but on an ARM macOS system that path hits. It would also hit on other BSDs or other OSes running on AArch64 but without /proc/cpuinfo.

For your reference, here is the error message from CMake for me:

CMake Error at /Users/chandlerc/src/llvm/llvm-project/openmp/runtime/cmake/LibompGetArchitecture.cmake:74 (file):
  file failed to open for reading (No such file or directory):

    /proc/cpuinfo
Call Stack (most recent call first):
  /Users/chandlerc/src/llvm/llvm-project/openmp/runtime/CMakeLists.txt:73 (libomp_is_aarch64_a64fx)

Will fix it right away.

I didn't think my CMake fu was up to it, but I think I have a fix, WDYT: https://reviews.llvm.org/D94889

And thanks to the speedy review, landed fix in: https://github.com/llvm/llvm-project/commit/f855751c1284c82c1c46b98f6d1b3ca2021d6cb9