[aarch64][patch] Enable GCC builtin intrinsics (__sync_*) on Clang

[David_A_3]

Hi,

Clang supports GCC builtin intrinsics (__sync_*) used for atomics. I tested master branch and Clang 3.5 in Fedora 21 on x86_64 and aarch64 machines. include/clang/Basic/Builtins.def from Clang contained every intrinsic used in TBB. I compiled TBB (current version, tbb43_20150316oss) on x86_64 with Clang with -DTBB_USE_GCC_BUILTINS=1  and it compiled. I seems Clang isn't fully maintained and some tests fail even on x86_64. Similar tests failed on aarch64 (plus one additional).

TBB headers are parsed by C++ interpreter (Cling) and that's there it failed for us on aarch64. Considering that Clang supports needed bits, I would suggest to enable use on GCC builtins in Clang too. Patch below. I enabled it for 3.5, because that's the oldest version I tried.

BTW, I was not able to login to TBB contribution page: The website encountered an unexpected error.

diff --git a/include/tbb/tbb_config.h b/include/tbb/tbb_config.h
index c65976c..567c36a 100644
--- a/include/tbb/tbb_config.h
+++ b/include/tbb/tbb_config.h
@@ -261,7 +261,7 @@
 /* Actually ICC supports gcc __sync_* intrinsics starting 11.1,
  * but 64 bit support for 32 bit target comes in later ones*/
 /* TODO: change the version back to 4.1.2 once macro __TBB_WORD_SIZE become optional */
-#if __TBB_GCC_VERSION >= 40306 || __INTEL_COMPILER >= 1200
+#if __TBB_GCC_VERSION >= 40306 || __INTEL_COMPILER >= 1200 || __TBB_CLANG_VERSION >= 30500
     /** built-in atomics available in GCC since 4.1.2 **/
     #define __TBB_GCC_BUILTIN_ATOMICS_PRESENT 1
 #endif
@@ -542,7 +542,7 @@
     #define __TBB_SSE_STACK_ALIGNMENT_BROKEN 0
 #endif

-#if __GNUC__==4 && __GNUC_MINOR__==3 && __GNUC_PATCHLEVEL__==0
+#if !defined(__clang__) && (__GNUC__==4 && __GNUC_MINOR__==3 && __GNUC_PATCHLEVEL__==0)
     /* GCC of this version may rashly ignore control dependencies */
     #define __TBB_GCC_OPTIMIZER_ORDERING_BROKEN 1
 #endif

[Jackson_M_Intel]

Hello,

Could you try submitting this again to the contribution page here https://www.threadingbuildingblocks.org/submit-contribution-form and if it breaks, let me know.

Thanks.

[David_A_3]

I done it some time ago, but did not receive any feedback/reviews regarding the patch. Is there an official repository there I could check that the patch is applied?

[RafSchietekat]

How is the support for '__atomic' coming along?

"These functions are intended to replace the legacy ‘__sync’ builtins."

[Vladimir_P_1234567890]

David A. wrote:

I done it some time ago, but did not receive any feedback/reviews regarding the patch. Is there an official repository there I could check that the patch is applied?

Hello, the patch is in review and have not passed it yet since __TBB_CLANG_VERSION macro is vendor-specific and does not depend on version tag. see more at http://clang.llvm.org/docs/LanguageExtensions.html

__clang_major__

Defined to the major marketing version number of Clang (e.g., the 2 in 2.0.1). Note that marketing version numbers should not be used to check for language features, as different vendors use different numbering schemes. Instead, use the Feature Checking Macros.

__clang_minor__

Defined to the minor version number of Clang (e.g., the 0 in 2.0.1). Note that marketing version numbers should not be used to check for language features, as different vendors use different numbering schemes. Instead, use the Feature Checking Macros.

[David_A_3]

A quick look suggests that '__atomic' are present in Clang (~3.1). IIUC, they are required to work with libstdc++. TBB itself does not use '__atomic_*' the last time I checked.

Regarding the __TBB_CLANG_VERSION is probably correct. Apple Clang latest version is 6.1.0 (still higher compared to open-source Clang version). At this point I don't see any macros for checking '__sync_*' availability. TBB does not use configure/make or CMake, thus you cannot check for their availability. Any compiler based on Clang could report different versions.

I could rewrite the patch, which on Clang would do checks for required '__sync_*' (there are 6-8 used in TBB IIRC) using __has_builtin and abort with an error if at least one is not supported.

Correct me if I am wrong on any of this points.

[Vladimir_P_1234567890]

TBB uses '__atomic_*' in machine layer for Intel Compiler so far. And you are correct here that for other compilers we need to understand which compiler runtime supports '__atomic_*' feature.

For the contribution the current idea is just enable built-in atomics for all versions of clang since it claims support of gcc 4.2.1 and __synch atomics appeared in gcc 4.1.2.

__has_builtin approach is not very good in this case since it it simple to add these 6 features but it would be quite hard to keep them in sync in 2 places in case something is changed or improved in the implementation.

--Vladimir

[David_A_3]

Has this landed in TBB already? I looked at tbb43_20150611oss include/tbb/tbb_config.h and seems that this is not yet in.

[Vladimir_P_1234567890]

hello David,

the patch was integrated in the build, should be available in next release.

--Vladimir

[Vladimir_P_1234567890]

released.

--Vladimir