Skip to content

[OpenMP][test][VE] Limit the number of threads to create #65873

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.

Sign up for GitHub

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

Closed
wants to merge 1 commit into from
Closed

[OpenMP][test][VE] Limit the number of threads to create #65873

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions openmp/runtime/src/kmp.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1153,8 +1153,15 @@ extern void __kmp_init_target_task();
#if defined(PTHREAD_THREADS_MAX) && PTHREAD_THREADS_MAX < INT_MAX
#define KMP_MAX_NTH PTHREAD_THREADS_MAX
#else
#ifdef __ve__
// VE's pthread supports only up to 64 threads per a VE process.
// https://sxauroratsubasa.sakura.ne.jp/documents/veos/en/VEOS_high_level_design.pdf
// p. 14 Maximum number of threads per VE process is 64.
#define KMP_MAX_NTH 64
#else
#define KMP_MAX_NTH INT_MAX
#endif
#endif
#endif /* KMP_MAX_NTH */

#ifdef PTHREAD_STACK_MIN
Expand Down
8 changes: 8 additions & 0 deletions openmp/runtime/src/z_Linux_util.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1894,13 +1894,21 @@ void __kmp_runtime_initialize(void) {

/* Query the maximum number of threads */
__kmp_type_convert(sysconf(_SC_THREAD_THREADS_MAX), &(__kmp_sys_max_nth));
#ifdef __ve__
if (__kmp_sys_max_nth == -1) {
// VE's pthread supports only up to 64 threads per a VE process.
// So we use that KMP_MAX_NTH (predefined as 64) here.
__kmp_sys_max_nth = KMP_MAX_NTH;
}
#else
if (__kmp_sys_max_nth == -1) {
/* Unlimited threads for NPTL */
__kmp_sys_max_nth = INT_MAX;
} else if (__kmp_sys_max_nth <= 1) {
/* Can't tell, just use PTHREAD_THREADS_MAX */
__kmp_sys_max_nth = KMP_MAX_NTH;
}
#endif

/* Query the minimum stack size */
__kmp_sys_min_stksize = sysconf(_SC_THREAD_STACK_MIN);
Expand Down
5 changes: 5 additions & 0 deletions openmp/runtime/test/atomic/omp-atomic-compare-signedness.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,12 @@
// UNSUPPORTED: gcc

// High parallelism increases our chances of detecting a lack of atomicity.
#ifdef __ve__
// VE's pthread_create supports 64 threads at a maximum.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If that's the case, then you want to set the limit (such as __kmp_sys_max_nth) in kmp.h as well.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks. Modifying initial value for __kmp_sys_max_nth is not enough for our cases since __kmp_runtime_initialize is implemented assuming NPTL which assumes infinity number of threads. I modify both KMP_MAX_NTH and the mechanism in __kmp_runtime_initialize.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It would be better to split the patch into two: one is about test, as the previous version, and one for the changes in the header, etc.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Will do. Thanks.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I created #66729 . I'm not sure whether I create another patch for test or not. Test itself passes after applying #66729 with following warnings. I can say this is good regression tests for #66729 . I also can say it's good to change test too for let ppl know this limitation. Do you have any opinion?

OMP: Warning #96: Cannot form a team with 256 threads, using 64 instead.
OMP: Hint Consider unsetting KMP_DEVICE_THREAD_LIMIT (KMP_ALL_THREADS), KMP_TEAMS_THREAD_LIMIT, and OMP_THREAD_LIMIT (if any are set).

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good to hear the test can pass after #66729. I'm more inclined to leave the test alone since we don't know what users can really do and the number looks totally reasonable from OpenMP's perspective. It is just our implementation can't handle it correctly.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That make sense too. I'll leave the test as is. Thank you for your advice.

#define NUM_THREADS_TRY 64
#else
#define NUM_THREADS_TRY 256
#endif

#include <limits.h>
#include <omp.h>
Expand Down