From e03383e612add869ea68efab5e1245cbd8d44f99 Mon Sep 17 00:00:00 2001 From: kwasimensah Date: Thu, 2 Nov 2017 15:35:59 -0400 Subject: [PATCH 1/2] Update support for buggy android NDK Expand logic for when __thread is supported to work around https://github.com/android-ndk/ndk/issues/8 --- src/LinearMath/btQuickprof.cpp | 74 ++++++++++++++++++++-------------- 1 file changed, 44 insertions(+), 30 deletions(-) diff --git a/src/LinearMath/btQuickprof.cpp b/src/LinearMath/btQuickprof.cpp index aed3104a6..8409ee3fe 100644 --- a/src/LinearMath/btQuickprof.cpp +++ b/src/LinearMath/btQuickprof.cpp @@ -680,46 +680,60 @@ void CProfileManager::dumpAll() CProfileManager::Release_Iterator(profileIterator); } +// BT_HAVE_TLS is defined to 1 when __thread should be supported. +// We assume __thread is supported on Linux when compiled with Clang or compiled +// against libstdc++ with _GLIBCXX_HAVE_TLS defined. +#ifdef BT_HAVE_TLS +#error BT_HAVE_TLS cannot be directly set +#elif defined(__linux__) && (defined(__clang__) || defined(_GLIBCXX_HAVE_TLS)) +#define BT_HAVE_TLS 1 +#endif - +// There are platforms for which TLS should not be used even though the compiler +// makes it seem like it's supported (Android NDK < r12b for example). +// This is primarily because of linker problems and toolchain misconfiguration: +// TLS isn't supported until NDK r12b per +// https://developer.android.com/ndk/downloads/revision_history.html +// Since NDK r16, `__NDK_MAJOR__` and `__NDK_MINOR__` are defined in +// . For NDK < r16, users should define these macros, +// e.g. `-D__NDK_MAJOR__=11 -D__NKD_MINOR__=0` for NDK r11. +#if defined(__ANDROID__) && defined(__clang__) +#if __has_include() +#include +#endif // __has_include() +#if defined(__ANDROID__) && defined(__clang__) && defined(__NDK_MAJOR__) && \ + defined(__NDK_MINOR__) && \ + ((__NDK_MAJOR__ < 12) || ((__NDK_MAJOR__ == 12) && (__NDK_MINOR__ < 1))) +#undef BT_HAVE_TLS +#endif +#endif // defined(__ANDROID__) && defined(__clang__) unsigned int btQuickprofGetCurrentThreadIndex2() { #if BT_THREADSAFE return btGetCurrentThreadIndex(); -#else // #if BT_THREADSAFE - const unsigned int kNullIndex = ~0U; +#else + const unsigned int kNullIndex = ~0U; #ifdef _WIN32 - #if defined(__MINGW32__) || defined(__MINGW64__) - static __thread unsigned int sThreadIndex = kNullIndex; - #else - __declspec( thread ) static unsigned int sThreadIndex = kNullIndex; - #endif +#if defined(__MINGW32__) || defined(__MINGW64__) + static __thread unsigned int sThreadIndex = kNullIndex; #else -#ifdef __APPLE__ - #if TARGET_OS_IPHONE - unsigned int sThreadIndex = 0; - return -1; - #else - static __thread unsigned int sThreadIndex = kNullIndex; - #endif -#else//__APPLE__ -#if __linux__ - static __thread unsigned int sThreadIndex = kNullIndex; + __declspec(thread) static unsigned int sThreadIndex = kNullIndex; +#endif +#elif defined(BT_HAVE_TLS) + static __thread unsigned int sThreadIndex = kNullIndex; #else - unsigned int sThreadIndex = 0; - return -1; -#endif -#endif//__APPLE__ - -#endif - static int gThreadCounter=0; + unsigned int sThreadIndex = 0; + return -1; +#endif //_WIN32 - if ( sThreadIndex == kNullIndex ) - { - sThreadIndex = gThreadCounter++; - } - return sThreadIndex; + // TODO: Use std::atomic? Not sure if c++11 is a prereq for this library. + static int gThreadCounter = 0; + + if (sThreadIndex == kNullIndex) { + sThreadIndex = gThreadCounter++; + } + return sThreadIndex; #endif // #else // #if BT_THREADSAFE } From 8aefa7e8d05118a09d9bb3a4c49db7678391b7b5 Mon Sep 17 00:00:00 2001 From: kwasimensah Date: Fri, 3 Nov 2017 11:59:12 -0400 Subject: [PATCH 2/2] Updating Macros Don't take away ability to do profiling on MacOS --- src/LinearMath/btQuickprof.cpp | 60 ++++++++++++++-------------------- 1 file changed, 24 insertions(+), 36 deletions(-) diff --git a/src/LinearMath/btQuickprof.cpp b/src/LinearMath/btQuickprof.cpp index 8409ee3fe..d06ddaf54 100644 --- a/src/LinearMath/btQuickprof.cpp +++ b/src/LinearMath/btQuickprof.cpp @@ -680,61 +680,49 @@ void CProfileManager::dumpAll() CProfileManager::Release_Iterator(profileIterator); } -// BT_HAVE_TLS is defined to 1 when __thread should be supported. -// We assume __thread is supported on Linux when compiled with Clang or compiled -// against libstdc++ with _GLIBCXX_HAVE_TLS defined. -#ifdef BT_HAVE_TLS -#error BT_HAVE_TLS cannot be directly set -#elif defined(__linux__) && (defined(__clang__) || defined(_GLIBCXX_HAVE_TLS)) -#define BT_HAVE_TLS 1 +// clang-format off +#if defined(_WIN32) && (defined(__MINGW32__) || defined(__MINGW64__)) + #define BT_HAVE_TLS 1 +#elif __APPLE__ && !TARGET_OS_IPHONE + // TODO: Modern versions of iOS support TLS now with updated version checking. + #define BT_HAVE_TLS 1 +#elif __linux__ + #define BT_HAVE_TLS 1 #endif -// There are platforms for which TLS should not be used even though the compiler -// makes it seem like it's supported (Android NDK < r12b for example). -// This is primarily because of linker problems and toolchain misconfiguration: -// TLS isn't supported until NDK r12b per -// https://developer.android.com/ndk/downloads/revision_history.html -// Since NDK r16, `__NDK_MAJOR__` and `__NDK_MINOR__` are defined in -// . For NDK < r16, users should define these macros, -// e.g. `-D__NDK_MAJOR__=11 -D__NKD_MINOR__=0` for NDK r11. +// __thread is broken on Andorid clang until r12b. See +// https://github.com/android-ndk/ndk/issues/8 #if defined(__ANDROID__) && defined(__clang__) -#if __has_include() -#include -#endif // __has_include() -#if defined(__ANDROID__) && defined(__clang__) && defined(__NDK_MAJOR__) && \ - defined(__NDK_MINOR__) && \ + #if __has_include() + #include + #endif // __has_include() + #if defined(__NDK_MAJOR__) && \ ((__NDK_MAJOR__ < 12) || ((__NDK_MAJOR__ == 12) && (__NDK_MINOR__ < 1))) -#undef BT_HAVE_TLS -#endif + #undef BT_HAVE_TLS + #endif #endif // defined(__ANDROID__) && defined(__clang__) +// clang-format on -unsigned int btQuickprofGetCurrentThreadIndex2() -{ -#if BT_THREADSAFE - return btGetCurrentThreadIndex(); -#else +unsigned int btQuickprofGetCurrentThreadIndex2() { const unsigned int kNullIndex = ~0U; -#ifdef _WIN32 -#if defined(__MINGW32__) || defined(__MINGW64__) - static __thread unsigned int sThreadIndex = kNullIndex; -#else - __declspec(thread) static unsigned int sThreadIndex = kNullIndex; -#endif + +#if BT_THREADSAFE + return btGetCurrentThreadIndex(); #elif defined(BT_HAVE_TLS) static __thread unsigned int sThreadIndex = kNullIndex; +#elif defined(_WIN32) + __declspec(thread) static unsigned int sThreadIndex = kNullIndex; #else unsigned int sThreadIndex = 0; return -1; -#endif //_WIN32 +#endif - // TODO: Use std::atomic? Not sure if c++11 is a prereq for this library. static int gThreadCounter = 0; if (sThreadIndex == kNullIndex) { sThreadIndex = gThreadCounter++; } return sThreadIndex; -#endif // #else // #if BT_THREADSAFE } void btEnterProfileZoneDefault(const char* name)