Hi,I have two problems with the function num_processors in libgnu's nproc.c.

FIRST PROBLEM

num_processors (NPROC_CURRENT_OVERRIDABLE) returns the value ofOMP_NUM_THREADS. However, if a thread limit is defined, it should returnthe minimum of OMP_NUM_THREADS and OMP_THREAD_LIMIT.

OMP_THREAD_LIMIT=1 OMP_NUM_THREADS=2 ./run-my-program- What I get: 2- What I expect: 1- Reason: The effective number of threads cannot exceed the thread limit.

SECOND PROBLEM

num_processors (NPROC_CURRENT_OVERRIDABLE) parses the environmentvariable OMP_NUM_THREADS for an integer. However, according to OpenMPdocumentation it may be a list of integers. In that case, the functionignores the value of OMP_NUM_THREADS. Seehttps://gcc.gnu.org/onlinedocs/libgomp/OMP_005fNUM_005fTHREADS.html

OMP_NUM_THREADS=2,2,1 ./run-my-program- What I get: 4- What I expect: 2 or 1 depending on the current OpenMP nesting level

Post by Oliver HeimlichFollow-Up: Savannah bugs #48534 #48535.Hi,I have two problems with the function num_processors in libgnu's nproc.c.FIRST PROBLEMnum_processors (NPROC_CURRENT_OVERRIDABLE) returns the value ofOMP_NUM_THREADS. However, if a thread limit is defined, it should returnthe minimum of OMP_NUM_THREADS and OMP_THREAD_LIMIT.OMP_THREAD_LIMIT=1 OMP_NUM_THREADS=2 ./run-my-program- What I get: 2- What I expect: 1- Reason: The effective number of threads cannot exceed the thread limit.SECOND PROBLEMnum_processors (NPROC_CURRENT_OVERRIDABLE) parses the environmentvariable OMP_NUM_THREADS for an integer. However, according to OpenMPdocumentation it may be a list of integers. In that case, the functionignores the value of OMP_NUM_THREADS. Seehttps://gcc.gnu.org/onlinedocs/libgomp/OMP_005fNUM_005fTHREADS.htmlOMP_NUM_THREADS=2,2,1 ./run-my-program- What I get: 4- What I expect: 2 or 1 depending on the current OpenMP nesting level

There is no way to determine the current nesting level,without actually using OpenMP, so I added supportfor calling omp_get_num_threads() if _OPENMP is defined.I'm not 100% sure it's valid to do that without depending onthe openmp module (which I think it's best not for nproc to do),so I'm open to removing that portion.

Also I added support for deferring to the first nesting levelin OMP_NUM_THREADS, and for OMP_THREADS_LIMIT like you suggest.

Post by Oliver HeimlichFollow-Up: Savannah bugs #48534 #48535.Hi,I have two problems with the function num_processors in libgnu's nproc.c.FIRST PROBLEMnum_processors (NPROC_CURRENT_OVERRIDABLE) returns the value ofOMP_NUM_THREADS. However, if a thread limit is defined, it should returnthe minimum of OMP_NUM_THREADS and OMP_THREAD_LIMIT.OMP_THREAD_LIMIT=1 OMP_NUM_THREADS=2 ./run-my-program- What I get: 2- What I expect: 1- Reason: The effective number of threads cannot exceed the thread limit.SECOND PROBLEMnum_processors (NPROC_CURRENT_OVERRIDABLE) parses the environmentvariable OMP_NUM_THREADS for an integer. However, according to OpenMPdocumentation it may be a list of integers. In that case, the functionignores the value of OMP_NUM_THREADS. Seehttps://gcc.gnu.org/onlinedocs/libgomp/OMP_005fNUM_005fTHREADS.htmlOMP_NUM_THREADS=2,2,1 ./run-my-program- What I get: 4- What I expect: 2 or 1 depending on the current OpenMP nesting level

There is no way to determine the current nesting level,without actually using OpenMP, so I added supportfor calling omp_get_num_threads() if _OPENMP is defined.I'm not 100% sure it's valid to do that without depending onthe openmp module (which I think it's best not for nproc to do),so I'm open to removing that portion.Also I added support for deferring to the first nesting levelin OMP_NUM_THREADS, and for OMP_THREADS_LIMIT like you suggest.

The function 'num_processors' is now a bit large, and in particular containsthe multiplied complexity of- the number of systems which each require a different approach,- the query parameter which in one MUST consider omp_env_limit and in theother case MAY but NEED NOT consider omp_env_limit.

To make this code more maintainable, I would propose to split it into- a function which contains only the complexity of the various systems,- a function which contains only the complexity of OMP handling.

Like this:

2017-02-26 Bruno Haible <***@clisp.org>

nproc: Refactor large function.* lib/nproc.c (num_processors_ignoring_omp): New function, extractedfrom num_processors.(num_processors): In this function, only deal with OMP.

Post by Bruno HaibleHi Pádraig,The function 'num_processors' is now a bit large, and in particular containsthe multiplied complexity of- the number of systems which each require a different approach,- the query parameter which in one MUST consider omp_env_limit and in theother case MAY but NEED NOT consider omp_env_limit.To make this code more maintainable, I would propose to split it into- a function which contains only the complexity of the various systems,- a function which contains only the complexity of OMP handling.nproc: Refactor large function.* lib/nproc.c (num_processors_ignoring_omp): New function, extractedfrom num_processors.(num_processors): In this function, only deal with OMP.diff --git a/lib/nproc.c b/lib/nproc.cindex db3ca9b..7880e62 100644--- a/lib/nproc.c+++ b/lib/nproc.c@@ -199,65 +199,12 @@ num_processors_via_affinity_mask (void)return 0;}-/* Parse OMP environment variables without dependence on OMP.- Return 0 for invalid values. */-static unsigned long int-parse_omp_threads (char const* threads)-{- unsigned long int ret = 0;-- if (threads == NULL)- return ret;-- /* The OpenMP spec says that the value assigned to the environment variables- "may have leading and trailing white space". */- while (*threads != '\0' && c_isspace (*threads))- threads++;- /* Convert it from positive decimal to 'unsigned long'. */- if (c_isdigit (*threads))- {- char *endptr = NULL;- unsigned long int value = strtoul (threads, &endptr, 10);-- if (endptr != NULL)- {- while (*endptr != '\0' && c_isspace (*endptr))- endptr++;- if (*endptr == '\0')- return value;- /* Also accept the first value in a nesting level,- since we can't determine the nesting level from env vars. */- else if (*endptr == ',')- return value;- }- }-- return ret;-}--unsigned long int-num_processors (enum nproc_query query)+/* Return the total number of processors. Here QUERY must be one of+ NPROC_ALL, NPROC_CURRENT. The result is guaranteed to be at least 1. */+static unsigned long int+num_processors_ignoring_omp (enum nproc_query query){- unsigned long int omp_env_limit = ULONG_MAX;-- if (query == NPROC_CURRENT_OVERRIDABLE)- {- unsigned long int omp_env_threads;- /* Honor the OpenMP environment variables, recognized also by all- programs that are based on OpenMP. */- omp_env_threads = parse_omp_threads (getenv ("OMP_NUM_THREADS"));- omp_env_limit = parse_omp_threads (getenv ("OMP_THREAD_LIMIT"));- if (! omp_env_limit)- omp_env_limit = ULONG_MAX;-- if (omp_env_threads)- return MIN (omp_env_threads, omp_env_limit);-- query = NPROC_CURRENT;- }- /* Here query is one of NPROC_ALL, NPROC_CURRENT. */-/* On systems with a modern affinity mask system call, we havesysconf (_SC_NPROCESSORS_CONF)