Allow overriding USE_COMPILER_TLS (formerly HAS_COMPILER_TLS).

[oon3m0oo]

This is useful, for example, for targets where glibc has deadlock issues with its built-in TLS implementation, as described in #1720.

[oon3m0oo]

Travis failures seem to get apt-get install issues...?

[martin-frbg]

Travis has been a bit unstable lately, restarting the failed jobs usually works.

[martin-frbg]

Not sure if that define gets passed through the Makefiles automatically, and perhaps USE_COMPILER_TLS should default to 0 on glibc systems ? (Is there any major advantage in using compiler vs pthreads tls where both are available ? I think so far we simply do not know if the lockups are limited to glibc, or if musl etc are similarly affected)

[oon3m0oo]

Actually, this whole thing is a bit fishy, I'm going to respond on the other thread.

[amurzeau]

I've added 2 comments about possible redefinition of USE_COMPILER_TLS in case of android and macos targets.

[amurzeau]

Same here: possible redefinition of USE_COMPILER_TLS as __clang__ is checked and will cause that define to be set to 1 at line 478.

[amurzeau]

Is it possible to have this condition match while also have a previous one match ?
This would mean a previous line (like 478) does #define USE_COMPILER_TLS 1 and then this one redefine it to 0 causing a compiler warning or error.
Maybe just add a #undef USE_COMPILER_TLS before this line.

[amurzeau]

I've compiled this patch and indeed, the build system need a modification to pass around USE_COMPILER_TLS.
I've applied this:

--- a/Makefile.system
+++ b/Makefile.system
@@ -1016,6 +1016,10 @@ ifdef USE_SIMPLE_THREADED_LEVEL3
 CCOMMON_OPT   += -DUSE_SIMPLE_THREADED_LEVEL3
 endif
 
+ifdef USE_COMPILER_TLS
+CCOMMON_OPT     += -DUSE_COMPILER_TLS=$(USE_COMPILER_TLS)
+endif
+
 ifndef SYMBOLPREFIX
 SYMBOLPREFIX =
 endif

Also, to try harder if the deadlock might occurs or not, I've made a small C test program that does a bunch of dlopen and dlclose and check in gdb if it runs or not (easy to see as gdb will show whenever a thread is created):

#include <dlfcn.h>
#include <stdio.h>

int main(int argc, char* argv[]) {
        while(1) {
                void* handle = dlopen(argv[1], RTLD_NOW);
                if(handle == 0) {
                        printf("Can't open openblas: %s\n", dlerror());
                        return 1;
                }
                dlclose(handle);
        }
        return 0;
}

And this patch + the additional change on the build system to support USE_COMPILER_TLS fix the deadlock. The full patch is here: https://salsa.debian.org/science-team/openblas/blob/e51b3b26641901a5a93247ef9bb9dbdddc82cfae/debian/patches/0008-Allow-overriding-USE_COMPILER_TLS-formerly-HAS_COMPI.patch
As an additional note, when compiling I got these warnings:

cc -Wdate-time -D_FORTIFY_SOURCE=2 -g -O2 -fdebug-prefix-map=/<<PKGBUILDDIR>>=. -fstack-protector-strong -Wformat -Werror=format-security -march=native -mtune=native  -DMAX_STACK_ALLOC=2048 -Wall -m64 -DF_INTERFACE_GFORT -fPIC -DNO_LAPACKE -DSMP_SERVER -DNO_WARMUP -DMAX_CPU_NUMBER=4 -DMAX_PARALLEL_NUMBER=1 -DUSE_COMPILER_TLS=0 -DASMNAME=z_abs -DASMFNAME=z_abs_ -DNAME=z_abs_ -DCNAME=z_abs -DCHAR_NAME=\"z_abs_\" -DCHAR_CNAME=\"z_abs\" -DNO_AFFINITY -I../.. -c -DDOUBLE abs.c -o z_abs.o
memory.c: In function 'get_num_procs':
memory.c:191:7: warning: unused variable 'n' [-Wunused-variable]
 int i,n;
       ^
memory.c:191:5: warning: unused variable 'i' [-Wunused-variable]
 int i,n;
     ^
memory.c: In function 'get_memory_table':
memory.c:542:32: warning: cast from pointer to integer of different size [-Wpointer-to-int-cast]
   int local_memory_table_pos = (int)pthread_getspecific(local_storage_key);
                                ^
memory.c:555:44: warning: cast to pointer from integer of different size [-Wint-to-pointer-cast]
     pthread_setspecific(local_storage_key, (void*)local_memory_table_pos);

[oon3m0oo]

We should probably pass this over in favor if #1739.

[rgommers]

We should probably pass this over in favor if #1739.

That, and the issue referenced in the PR description was closed 6 years ago. I'm not even sure that a build config option to change TLS handling is desirable - it's one more variation to test, and it's better to properly fix TLS issues if there are any left in 2024 (many projects use thread-local storage, and it's a hard necessity usually - just turning it off isn't really an option).

Probably best to close this PR?

[martin-frbg]

Both these PRs have been cherry-picked for improvements ages ago, I had only kept them open for reference. If you think they should be closed to make the PR list look nicer, this can be done.

[rgommers]

Ah if they're merged, I would indeed close them.