<html><head><meta http-equiv="Content-Type" content="text/html charset=utf-8"></head><body style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space;" class=""><br class=""><div><blockquote type="cite" class=""><div class="">On Aug 11, 2016, at 9:02 PM, Saleem Abdulrasool <<a href="mailto:compnerd@compnerd.org" class="">compnerd@compnerd.org</a>> wrote:</div><br class="Apple-interchange-newline"><div class=""><div dir="ltr" style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class="">On Thu, Aug 11, 2016 at 6:29 PM, Chris Bieneman via llvm-commits<span class="Apple-converted-space"> </span><span dir="ltr" class=""><<a href="mailto:llvm-commits@lists.llvm.org" target="_blank" class="">llvm-commits@lists.llvm.org</a>></span><span class="Apple-converted-space"> </span>wrote:<br class=""><div class="gmail_extra"><div class="gmail_quote"><blockquote class="gmail_quote" style="margin: 0px 0px 0px 0.8ex; border-left-width: 1px; border-left-color: rgb(204, 204, 204); border-left-style: solid; padding-left: 1ex;">Author: cbieneman<br class="">Date: Thu Aug 11 20:29:26 2016<br class="">New Revision: 278454<br class=""><br class="">URL:<span class="Apple-converted-space"> </span><a href="http://llvm.org/viewvc/llvm-project?rev=278454&view=rev" rel="noreferrer" target="_blank" class="">http://llvm.org/viewvc/llvm-<wbr class="">project?rev=278454&view=rev</a><br class="">Log:<br class="">[CMake] If the compiler supports _Atomic include atomic.c in builtins libraries<br class=""><br class="">This fixes a long-standing TODO by implementing a compiler check for supporting the _Atomic keyword. If the _Atomic keyword is supported by the compiler we should include it in the builtin library sources.<br class=""><br class="">Modified:<br class=""> <span class="Apple-converted-space"> </span>compiler-rt/trunk/cmake/<wbr class="">Modules/BuiltinTests.cmake<br class=""> <span class="Apple-converted-space"> </span>compiler-rt/trunk/cmake/<wbr class="">builtin-config-ix.cmake<br class=""> <span class="Apple-converted-space"> </span>compiler-rt/trunk/lib/<wbr class="">builtins/CMakeLists.txt<br class=""><br class="">Modified: compiler-rt/trunk/cmake/<wbr class="">Modules/BuiltinTests.cmake<br class="">URL:<span class="Apple-converted-space"> </span><a href="http://llvm.org/viewvc/llvm-project/compiler-rt/trunk/cmake/Modules/BuiltinTests.cmake?rev=278454&r1=278453&r2=278454&view=diff" rel="noreferrer" target="_blank" class="">http://llvm.org/viewvc/llvm-<wbr class="">project/compiler-rt/trunk/<wbr class="">cmake/Modules/BuiltinTests.<wbr class="">cmake?rev=278454&r1=278453&r2=<wbr class="">278454&view=diff</a><br class="">==============================<wbr class="">==============================<wbr class="">==================<br class="">--- compiler-rt/trunk/cmake/<wbr class="">Modules/BuiltinTests.cmake (original)<br class="">+++ compiler-rt/trunk/cmake/<wbr class="">Modules/BuiltinTests.cmake Thu Aug 11 20:29:26 2016<br class="">@@ -3,11 +3,15 @@ include(<wbr class="">CMakeCheckCompilerFlagCommonPa<wbr class="">tt<br class=""> # This function takes an OS and a list of architectures and identifies the<br class=""> # subset of the architectures list that the installed toolchain can target.<br class=""> function(try_compile_only output)<br class="">+ cmake_parse_arguments(ARG "" "" "SOURCE;FLAGS" ${ARGN})<br class="">+ if(NOT ARG_SOURCE)<br class="">+ set(ARG_SOURCE "int foo(int x, int y) { return x + y; }\n")<br class="">+ endif()<br class=""> set(SIMPLE_C ${CMAKE_BINARY_DIR}${CMAKE_<wbr class="">FILES_DIRECTORY}/src.c)<br class="">- file(WRITE ${SIMPLE_C} "int foo(int x, int y) { return x + y; }\n")<br class="">+ file(WRITE ${SIMPLE_C} "${ARG_SOURCE}\n")<br class=""> string(REGEX MATCHALL "<[A-Za-z0-9_]*>" substitutions<br class=""> <span class="Apple-converted-space"> </span>${CMAKE_C_COMPILE_OBJECT})<br class="">- string(REPLACE ";" " " extra_flags "${ARGN}")<br class="">+ string(REPLACE ";" " " extra_flags "${ARG_FLAGS}")<br class=""><br class=""> set(test_compile_command "${CMAKE_C_COMPILE_OBJECT}")<br class=""> foreach(substitution ${substitutions})<br class="">@@ -63,7 +67,20 @@ endfunction()<br class=""> function(builtin_check_c_<wbr class="">compiler_flag flag output)<br class=""> if(NOT DEFINED ${output})<br class=""> message(STATUS "Performing Test ${output}")<br class="">- try_compile_only(result ${flag})<br class="">+ try_compile_only(result FLAGS ${flag})<br class="">+ set(${output} ${result} CACHE INTERNAL "Compiler supports ${flag}")<br class="">+ if(${result})<br class="">+ message(STATUS "Performing Test ${output} - Success")<br class="">+ else()<br class="">+ message(STATUS "Performing Test ${output} - Failed")<br class="">+ endif()<br class="">+ endif()<br class="">+endfunction()<br class="">+<br class="">+function(builtin_check_c_<wbr class="">compiler_source output source)<br class="">+ if(NOT DEFINED ${output})<br class="">+ message(STATUS "Performing Test ${output}")<br class="">+ try_compile_only(result SOURCE ${source})<br class=""> set(${output} ${result} CACHE INTERNAL "Compiler supports ${flag}")<br class=""> if(${result})<br class=""> message(STATUS "Performing Test ${output} - Success")<br class=""><br class="">Modified: compiler-rt/trunk/cmake/<wbr class="">builtin-config-ix.cmake<br class="">URL:<span class="Apple-converted-space"> </span><a href="http://llvm.org/viewvc/llvm-project/compiler-rt/trunk/cmake/builtin-config-ix.cmake?rev=278454&r1=278453&r2=278454&view=diff" rel="noreferrer" target="_blank" class="">http://llvm.org/viewvc/llvm-<wbr class="">project/compiler-rt/trunk/<wbr class="">cmake/builtin-config-ix.cmake?<wbr class="">rev=278454&r1=278453&r2=<wbr class="">278454&view=diff</a><br class="">==============================<wbr class="">==============================<wbr class="">==================<br class="">--- compiler-rt/trunk/cmake/<wbr class="">builtin-config-ix.cmake (original)<br class="">+++ compiler-rt/trunk/cmake/<wbr class="">builtin-config-ix.cmake Thu Aug 11 20:29:26 2016<br class="">@@ -1,4 +1,5 @@<br class=""> include(BuiltinTests)<br class="">+include(CheckCSourceCompiles)<br class=""><br class=""> # Make all the tests only check the compiler<br class=""> set(TEST_COMPILE_ONLY On)<br class="">@@ -14,6 +15,15 @@ builtin_check_c_compiler_flag(<wbr class="">-mfloat-ab<br class=""> builtin_check_c_compiler_flag(<wbr class="">-mfloat-abi=hard COMPILER_RT_HAS_FLOAT_ABI_<wbr class="">HARD_FLAG)<br class=""> builtin_check_c_compiler_flag(<wbr class="">-static COMPILER_RT_HAS_STATIC_FLAG)<br class=""><br class="">+builtin_check_c_compiler_<wbr class="">source(COMPILER_RT_SUPPORTS_<wbr class="">ATOMIC_KEYWORD<br class=""></blockquote><div class=""><br class=""></div><div class="">Probably better to use `COMPILER_RT_CC_SUPPORTS_ATOMIC_KEYWORD` as its not compiler-rt but rather CC that we are checking has support for the atomic keyword.</div></div></div></div></div></blockquote><div><br class=""></div><div>So, I agree that the option is incorrectly named, but I don’t agree with your suggestion. I think it should follow the convention of the other compiler checks in the project, which would be COMPILER_RT_HAS_ATOMIC_KEYWORD.</div><div><br class=""></div><div>Because of the convention we use where variable names start with the project name I think we need to try not to be overly verbose in naming. In places where we’re not we already end up with really crazy long lines.</div><div><br class=""></div><div>Thoughts?</div><div>-Chris</div><br class=""><blockquote type="cite" class=""><div class=""><div dir="ltr" style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class=""><div class="gmail_extra"><div class="gmail_quote"><div class=""> </div><blockquote class="gmail_quote" style="margin: 0px 0px 0px 0.8ex; border-left-width: 1px; border-left-color: rgb(204, 204, 204); border-left-style: solid; padding-left: 1ex;">+"<br class="">+int foo(int x, int y) {<br class="">+ _Atomic int result = x * y;<br class="">+ return result;<br class="">+}<br class="">+")<br class="">+<br class="">+<br class=""> set(ARM64 aarch64)<br class=""> set(ARM32 arm armhf)<br class=""> set(X86 i386 i686)<br class=""><br class="">Modified: compiler-rt/trunk/lib/<wbr class="">builtins/CMakeLists.txt<br class="">URL:<span class="Apple-converted-space"> </span><a href="http://llvm.org/viewvc/llvm-project/compiler-rt/trunk/lib/builtins/CMakeLists.txt?rev=278454&r1=278453&r2=278454&view=diff" rel="noreferrer" target="_blank" class="">http://llvm.org/viewvc/llvm-<wbr class="">project/compiler-rt/trunk/lib/<wbr class="">builtins/CMakeLists.txt?rev=<wbr class="">278454&r1=278453&r2=278454&<wbr class="">view=diff</a><br class="">==============================<wbr class="">==============================<wbr class="">==================<br class="">--- compiler-rt/trunk/lib/<wbr class="">builtins/CMakeLists.txt (original)<br class="">+++ compiler-rt/trunk/lib/<wbr class="">builtins/CMakeLists.txt Thu Aug 11 20:29:26 2016<br class="">@@ -42,8 +42,6 @@ set(GENERIC_SOURCES<br class=""> ashlti3.c<br class=""> ashrdi3.c<br class=""> ashrti3.c<br class="">- # FIXME: atomic.c may only be compiled if host compiler understands _Atomic<br class="">- # atomic.c<br class=""> clear_cache.c<br class=""> clzdi2.c<br class=""> clzsi2.c<br class="">@@ -166,6 +164,12 @@ set(GENERIC_SOURCES<br class=""> umodsi3.c<br class=""> umodti3.c)<br class=""><br class="">+if(COMPILER_RT_SUPPORTS_<wbr class="">ATOMIC_KEYWORD)<br class="">+ set(GENERIC_SOURCES<br class="">+ ${GENERIC_SOURCES}<br class="">+ atomic.c)<br class="">+endif()<br class="">+<br class=""> set(MSVC_SOURCES<br class=""> <span class="Apple-converted-space"> </span>divsc3.c<br class=""> <span class="Apple-converted-space"> </span>divdc3.c<br class=""><br class=""><br class="">______________________________<wbr class="">_________________<br class="">llvm-commits mailing list<br class=""><a href="mailto:llvm-commits@lists.llvm.org" class="">llvm-commits@lists.llvm.org</a><br class=""><a href="http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits" rel="noreferrer" target="_blank" class="">http://lists.llvm.org/cgi-bin/<wbr class="">mailman/listinfo/llvm-commits</a><br class=""></blockquote></div><br class=""><br clear="all" class=""><div class=""><br class=""></div>--<span class="Apple-converted-space"> </span><br class=""><div class="gmail_signature" data-smartmail="gmail_signature">Saleem Abdulrasool<br class="">compnerd (at) compnerd (dot) org</div></div></div></div></blockquote></div><br class=""></body></html>