[Openmp-commits] [PATCH] D22365: Make balanced affinity work on AArch64 (and possibly other architectures too)

[Paul Osmialowski via Openmp-commits]

pawosm01 added a comment.

There are ARMv8 boards where cores of one SoC are clustered into two packages: one for LITTLE cores and one for big. With original limitations this affinity method refused to work (due to nPackages > 1) which is bit discriminatory.

Note that I tested both uniform (equal number of cores per clusters) and non-uniform (more LITTLE cores than big) arrangements.

What I couldn't test (but I may gain access to such x86_64 machine in the future) is multi-package hyper-threaded machine (more than one CPU on board, each with more than one core of which each has more than one hardware thread) - I need to check again what effect my chagnes have on such arrangement (e.g. possible overvrites in procarr array).


================
Comment at: runtime/src/kmp_affinity.cpp:3994
@@ -3988,3 +3993,3 @@
             }
             int ncores = address2os[ __kmp_avail_proc - 1 ].first.labels[ core_level ] + 1;
             int nproc = nth_per_core * ncores;
----------------
AndreyChurbanov wrote:
> This is number of cores in the last package, as the code supposed to be run on single package topology only.
The whole point of change in line 3967 is to lift the limitation that the code is supposed to be run on single package topology only.


================
Comment at: runtime/src/kmp_affinity.cpp:3995
@@ -3989,3 +3994,3 @@
             int ncores = address2os[ __kmp_avail_proc - 1 ].first.labels[ core_level ] + 1;
             int nproc = nth_per_core * ncores;
 
----------------
AndreyChurbanov wrote:
> This code does not take into account number of packages (supposed to be run on single-package topology).
In case of nPackages > 1 and nCoresPerPkg, nth_per_core equals to nCoresPerPkg.
I did not change the algorithm, I didn't change names of the variables used either.


================
Comment at: runtime/src/kmp_affinity.cpp:4655
@@ -4621,3 +4654,3 @@
         // Number of cores - maximum value; it does not count trail cores with 0 processors
         int ncores = address2os[ __kmp_avail_proc - 1 ].first.labels[ core_level ] + 1;
 
----------------
AndreyChurbanov wrote:
> This is the number of cores in the last package. Other packages are skipped (no threads bound there).
Do you refer to the effect this has on line 4679? This code crashes when __kmp_ncores is used there instead.


Repository:
  rL LLVM

https://reviews.llvm.org/D22365