Simplify num_cpu_avail() syntax

[nh2]

• Based on Make OpenBLAS's usage of OpenMP respect openblas_set_num_threads() #5808, merge that first.
• Get @martin-frbg clarification on comment from Make OpenBLAS's usage of OpenMP respect openblas_set_num_threads() #5808 (comment)

  so nothing to do with forcing OMP parallel jobs inside an OMP_PARALLEL region

[nh2]

@martin-frbg answering your #5808 (comment)

Refactoring num_cpu_avail() may be a good idea at some point, but it bears the risk of introducing new bugs and I have neither the time nor energy to review that now.

I added back 1 line I had originally removed, which simplifies the review.

The change is purely syntactical (beyond relying on omp_get_max_threads() having no side effect).

For review convenience:

Step-by-step equivalence for num_cpu_avail() refactor

Shows that the change is purely syntactical (beyond relying on omp_get_max_threads() having no side effect).

Starting point (old code)

#ifdef USE_OPENMP
  /* ...unchanged prelude... */

int openmp_nthreads;
	openmp_nthreads=omp_get_max_threads();
	if (omp_in_parallel()) openmp_nthreads = blas_omp_threads_local;
#endif

#ifndef USE_OPENMP
  if (blas_cpu_number == 1
#else
     if (openmp_nthreads == 1
#endif
      ) return 1;

#ifdef USE_OPENMP
     if (openmp_nthreads > blas_omp_number_max){
     /* fprintf under #ifdef DEBUG */
     openmp_nthreads = blas_omp_number_max;
     }
     if (blas_cpu_number != openmp_nthreads) {
	  goto_set_num_threads(openmp_nthreads);
  }
#endif

  return blas_cpu_number;

To make review easier, split into two independent cases (one for USE_OPENMP defined, one not), then rejoin them at the end.

---

Path A: USE_OPENMP is NOT defined

Applying preprocessor:

  if (blas_cpu_number == 1
      ) return 1;

  return blas_cpu_number;

The early return 1; is redundant:

  return blas_cpu_number;

Path B: USE_OPENMP IS defined

Applying preprocessor:

int openmp_nthreads;
	openmp_nthreads=omp_get_max_threads();
	if (omp_in_parallel()) openmp_nthreads = blas_omp_threads_local;

     if (openmp_nthreads == 1
      ) return 1;

     if (openmp_nthreads > blas_omp_number_max){
       /* fprintf under #ifdef DEBUG */
     openmp_nthreads = blas_omp_number_max;
     }
     if (blas_cpu_number != openmp_nthreads) {
	  goto_set_num_threads(openmp_nthreads);
  }

  return blas_cpu_number;

Turn first 3 lines into ternary; no change since omp_get_max_threads() has no side effect.
Also normalise whitespace:

  int openmp_nthreads = omp_in_parallel() ? blas_omp_threads_local : omp_get_max_threads();

  if (openmp_nthreads == 1) return 1;

  if (openmp_nthreads > blas_omp_number_max) {
    /* fprintf under #ifdef DEBUG */
    openmp_nthreads = blas_omp_number_max;
  }
  if (blas_cpu_number != openmp_nthreads) {
    goto_set_num_threads(openmp_nthreads);
  }

  return blas_cpu_number;

Rejoin

Joining the results of A and B:

#ifdef USE_OPENMP
  /* ...unchanged prelude... */

  int openmp_nthreads = omp_in_parallel() ? blas_omp_threads_local : omp_get_max_threads();

  if (openmp_nthreads == 1) return 1;

  if (openmp_nthreads > blas_omp_number_max) {
    /* fprintf under #ifdef DEBUG */
    openmp_nthreads = blas_omp_number_max;
  }

  if (blas_cpu_number != openmp_nthreads) {
    goto_set_num_threads(openmp_nthreads);
  }
#endif

  return blas_cpu_number;

[itamarst]

@nh2 https://www.intel.com/content/www/us/en/docs/onemkl/developer-reference-c/2023-0/mkl-set-num-threads-local.html is I think the intent (not the reality) of what openblas_set_num_threads_local is supposed to do: instead of setting the number of threads for the whole process, it should set them for the current thread only. So it should work for non-OpenMP backends too, it's not an OpenMP thing. The implementation would involve thread locals.

This would be extremely useful; I expect to write an article about why soon, but the short version is that if user code is creating its own threads, only having a process-wide setting is very frustrating.

[nh2]

@itamarst Ah, OK. Well, th function really doesn't do any of that, not using thread-local variables at all.

I updated the comment with a write-up of that, so that outside contributors or users at least have an idea of what's going on.