-
Notifications
You must be signed in to change notification settings - Fork 12.6k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[openmp] Use core_siblings_list if physical_package_id not available #111831
Conversation
@@ -25,7 +25,7 @@ static int compare_hw_subset_places(const place_list_t *openmp_places, | |||
expected_per_place = nthreads_per_core; | |||
} else { | |||
expected_total = nsockets; | |||
expected_per_place = ncores_per_socket; | |||
expected_per_place = ncores_per_socket * nthreads_per_core; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Unless I'm misunderstanding something, the count should always be in terms of threads. I think maybe this test has been getting away with it, because on x86 the number of threads per core is at most 2, so after halving it it is always 1 and this multiplication does not matter. On the ppc system I'm testing the number of threads per core is 6, so after halving it's 3 and the test would fail if we don't multiply here.
✅ With the latest revision this PR passed the C/C++ code formatter. |
261649d
to
c4cedb2
Compare
On powerpc, physical_package_id may not be available. Currently, this causes openmp to fall back to flat topology and various affinity tests fail. Fix this by parsing core_siblings_list to deterimine which cpus belong to the same socket. This matches what the testing code does. The code to parse the CPU list format thankfully already exists. Fixes llvm#111809.
c4cedb2
to
5fb4d7f
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/73/builds/7004 Here is the relevant piece of the build log for the reference
|
…lvm#111831) On powerpc, physical_package_id may not be available. Currently, this causes openmp to fall back to flat topology and various affinity tests fail. Fix this by parsing core_siblings_list to deterimine which cpus belong to the same socket. This matches what the testing code does. The code to parse the CPU list format thankfully already exists. Fixes llvm#111809.
…lvm#111831) On powerpc, physical_package_id may not be available. Currently, this causes openmp to fall back to flat topology and various affinity tests fail. Fix this by parsing core_siblings_list to deterimine which cpus belong to the same socket. This matches what the testing code does. The code to parse the CPU list format thankfully already exists. Fixes llvm#111809.
…lvm#111831) On powerpc, physical_package_id may not be available. Currently, this causes openmp to fall back to flat topology and various affinity tests fail. Fix this by parsing core_siblings_list to deterimine which cpus belong to the same socket. This matches what the testing code does. The code to parse the CPU list format thankfully already exists. Fixes llvm#111809.
Backport patch from llvm/llvm-project#111831 to fix openmp affinity tests on some brew ppc runners. Also add one more openmp test to the ignore list for s390x. Also add --time-tests to the lit args -- I had one instance where openmp tests were running for 4h on s390x. If this happends again, this should help determine which tests take so much time.
On powerpc, physical_package_id may not be available. Currently, this causes openmp to fall back to flat topology and various affinity tests fail.
Fix this by parsing core_siblings_list to deterimine which cpus belong to the same socket. This matches what the testing code does. The code to parse the CPU list format thankfully already exists.
Fixes #111809.