[Openmp-commits] [PATCH] D22365: Make balanced affinity work on AArch64 (and possibly other architectures too)
Andrey Churbanov via Openmp-commits
openmp-commits at lists.llvm.org
Thu Jul 21 12:31:15 PDT 2016
AndreyChurbanov added inline comments.
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.
My comment was probably irrelevant here. The problem was incorrect filling of the procarr array that only contained info on the last package. So if this array is in use (I mostly tested case on line 4710), then all threads were bound to this last package.
If you fix memory overwriting at initialization, then anyway ncores will need to be multiplied by number of packages here or extra loop over packages needs to be added, I think. Otherwise l don't see how all machine can be covered, given that loops for( int i = 0; i < ncores; i++ ) only walk through subset of all cores now.
BTW, I finished with debugging of no-HT case. It works fine because you effectively reduced this case to single package via shifting topology one level down and making the balanced affinity to see packages as cores and cores as threads. This trick does not work for HT enabled topology.
More information about the Openmp-commits