<div dir="ltr">On Fri, Aug 12, 2016 at 9:39 AM, Chris Bieneman <span dir="ltr"><<a href="mailto:beanz@apple.com" target="_blank">beanz@apple.com</a>></span> wrote:<br><div class="gmail_extra"><div class="gmail_quote"><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div style="word-wrap:break-word"><br><div><div><div class="h5"><blockquote type="cite"><div>On Aug 11, 2016, at 9:02 PM, Saleem Abdulrasool <<a href="mailto:compnerd@compnerd.org" target="_blank">compnerd@compnerd.org</a>> wrote:</div><br><div><div dir="ltr" style="font-family:Helvetica;font-size:12px;font-style:normal;font-weight:normal;letter-spacing:normal;text-align:start;text-indent:0px;text-transform:none;white-space:normal;word-spacing:0px">On Thu, Aug 11, 2016 at 6:29 PM, Chris Bieneman via llvm-commits<span> </span><span dir="ltr"><<a href="mailto:llvm-commits@lists.llvm.org" target="_blank">llvm-commits@<wbr>lists.llvm.org</a>></span><span> </span>wrote:<br><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>Date: Thu Aug 11 20:29:26 2016<br>New Revision: 278454<br><br>URL:<span> </span><a href="http://llvm.org/viewvc/llvm-project?rev=278454&view=rev" rel="noreferrer" target="_blank">http://llvm.org/viewvc/<wbr>llvm-project?rev=278454&view=<wbr>rev</a><br>Log:<br>[CMake] If the compiler supports _Atomic include atomic.c in builtins libraries<br><br>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><br>Modified:<br> <span> </span>compiler-rt/trunk/cmake/Modu<wbr>les/BuiltinTests.cmake<br> <span> </span>compiler-rt/trunk/cmake/buil<wbr>tin-config-ix.cmake<br> <span> </span>compiler-rt/trunk/lib/builti<wbr>ns/CMakeLists.txt<br><br>Modified: compiler-rt/trunk/cmake/Module<wbr>s/BuiltinTests.cmake<br>URL:<span> </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">http://llvm.org/viewvc/<wbr>llvm-project/compiler-rt/<wbr>trunk/cmake/Modules/<wbr>BuiltinTests.cmake?rev=278454&<wbr>r1=278453&r2=278454&view=diff</a><br>==============================<wbr>==============================<wbr>==================<br>--- compiler-rt/trunk/cmake/Module<wbr>s/BuiltinTests.cmake (original)<br>+++ compiler-rt/trunk/cmake/Module<wbr>s/BuiltinTests.cmake Thu Aug 11 20:29:26 2016<br>@@ -3,11 +3,15 @@ include(CMakeCheckCompilerFlag<wbr>CommonPatt<br> # This function takes an OS and a list of architectures and identifies the<br> # subset of the architectures list that the installed toolchain can target.<br> function(try_compile_only output)<br>+ cmake_parse_arguments(ARG "" "" "SOURCE;FLAGS" ${ARGN})<br>+ if(NOT ARG_SOURCE)<br>+ set(ARG_SOURCE "int foo(int x, int y) { return x + y; }\n")<br>+ endif()<br> set(SIMPLE_C ${CMAKE_BINARY_DIR}${CMAKE_FIL<wbr>ES_DIRECTORY}/src.c)<br>- file(WRITE ${SIMPLE_C} "int foo(int x, int y) { return x + y; }\n")<br>+ file(WRITE ${SIMPLE_C} "${ARG_SOURCE}\n")<br> string(REGEX MATCHALL "<[A-Za-z0-9_]*>" substitutions<br> <span> </span>${CMAKE_C_COMPILE_OBJECT})<br>- string(REPLACE ";" " " extra_flags "${ARGN}")<br>+ string(REPLACE ";" " " extra_flags "${ARG_FLAGS}")<br><br> set(test_compile_command "${CMAKE_C_COMPILE_OBJECT}")<br> foreach(substitution ${substitutions})<br>@@ -63,7 +67,20 @@ endfunction()<br> function(builtin_check_c_comp<wbr>iler_flag flag output)<br> if(NOT DEFINED ${output})<br> message(STATUS "Performing Test ${output}")<br>- try_compile_only(result ${flag})<br>+ try_compile_only(result FLAGS ${flag})<br>+ set(${output} ${result} CACHE INTERNAL "Compiler supports ${flag}")<br>+ if(${result})<br>+ message(STATUS "Performing Test ${output} - Success")<br>+ else()<br>+ message(STATUS "Performing Test ${output} - Failed")<br>+ endif()<br>+ endif()<br>+endfunction()<br>+<br>+function(builtin_check_c_comp<wbr>iler_source output source)<br>+ if(NOT DEFINED ${output})<br>+ message(STATUS "Performing Test ${output}")<br>+ try_compile_only(result SOURCE ${source})<br> set(${output} ${result} CACHE INTERNAL "Compiler supports ${flag}")<br> if(${result})<br> message(STATUS "Performing Test ${output} - Success")<br><br>Modified: compiler-rt/trunk/cmake/builti<wbr>n-config-ix.cmake<br>URL:<span> </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">http://llvm.org/viewvc/<wbr>llvm-project/compiler-rt/<wbr>trunk/cmake/builtin-config-ix.<wbr>cmake?rev=278454&r1=278453&r2=<wbr>278454&view=diff</a><br>==============================<wbr>==============================<wbr>==================<br>--- compiler-rt/trunk/cmake/builti<wbr>n-config-ix.cmake (original)<br>+++ compiler-rt/trunk/cmake/builti<wbr>n-config-ix.cmake Thu Aug 11 20:29:26 2016<br>@@ -1,4 +1,5 @@<br> include(BuiltinTests)<br>+include(CheckCSourceCompiles)<br><br> # Make all the tests only check the compiler<br> set(TEST_COMPILE_ONLY On)<br>@@ -14,6 +15,15 @@ builtin_check_c_compiler_flag(<wbr>-mfloat-ab<br> builtin_check_c_compiler_<wbr>flag(-mfloat-abi=hard COMPILER_RT_HAS_FLOAT_ABI_HARD<wbr>_FLAG)<br> builtin_check_c_compiler_<wbr>flag(-static COMPILER_RT_HAS_STATIC_FLAG)<br><br>+builtin_check_c_compiler_sour<wbr>ce(COMPILER_RT_SUPPORTS_ATOMIC<wbr>_KEYWORD<br></blockquote><div><br></div><div>Probably better to use `COMPILER_RT_CC_SUPPORTS_<wbr>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></div></div></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_<wbr>KEYWORD.</div></div></div></blockquote><div><br></div><div>Sounds reasonable enough to me.</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div style="word-wrap:break-word"><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></div><div>Thoughts?</div><span class="HOEnZb"><font color="#888888"><div>-Chris</div></font></span><div><div class="h5"><br><blockquote type="cite"><div><div dir="ltr" style="font-family:Helvetica;font-size:12px;font-style:normal;font-weight:normal;letter-spacing:normal;text-align:start;text-indent:0px;text-transform:none;white-space:normal;word-spacing:0px"><div class="gmail_extra"><div class="gmail_quote"><div> </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>+int foo(int x, int y) {<br>+ _Atomic int result = x * y;<br>+ return result;<br>+}<br>+")<br>+<br>+<br> set(ARM64 aarch64)<br> set(ARM32 arm armhf)<br> set(X86 i386 i686)<br><br>Modified: compiler-rt/trunk/lib/builtins<wbr>/CMakeLists.txt<br>URL:<span> </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">http://llvm.org/viewvc/<wbr>llvm-project/compiler-rt/<wbr>trunk/lib/builtins/CMakeLists.<wbr>txt?rev=278454&r1=278453&r2=<wbr>278454&view=diff</a><br>==============================<wbr>==============================<wbr>==================<br>--- compiler-rt/trunk/lib/builtins<wbr>/CMakeLists.txt (original)<br>+++ compiler-rt/trunk/lib/builtins<wbr>/CMakeLists.txt Thu Aug 11 20:29:26 2016<br>@@ -42,8 +42,6 @@ set(GENERIC_SOURCES<br> ashlti3.c<br> ashrdi3.c<br> ashrti3.c<br>- # FIXME: atomic.c may only be compiled if host compiler understands _Atomic<br>- # atomic.c<br> clear_cache.c<br> clzdi2.c<br> clzsi2.c<br>@@ -166,6 +164,12 @@ set(GENERIC_SOURCES<br> umodsi3.c<br> umodti3.c)<br><br>+if(COMPILER_RT_SUPPORTS_ATOMI<wbr>C_KEYWORD)<br>+ set(GENERIC_SOURCES<br>+ ${GENERIC_SOURCES}<br>+ atomic.c)<br>+endif()<br>+<br> set(MSVC_SOURCES<br> <span> </span>divsc3.c<br> <span> </span>divdc3.c<br><br><br>______________________________<wbr>_________________<br>llvm-commits mailing list<br><a href="mailto:llvm-commits@lists.llvm.org" target="_blank">llvm-commits@lists.llvm.org</a><br><a href="http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits" rel="noreferrer" target="_blank">http://lists.llvm.org/cgi-bin/<wbr>mailman/listinfo/llvm-commits</a><br></blockquote></div><br><br clear="all"><div><br></div>--<span> </span><br><div data-smartmail="gmail_signature">Saleem Abdulrasool<br>compnerd (at) compnerd (dot) org</div></div></div></div></blockquote></div></div></div><br></div></blockquote></div><br><br clear="all"><div><br></div>-- <br><div class="gmail_signature" data-smartmail="gmail_signature">Saleem Abdulrasool<br>compnerd (at) compnerd (dot) org</div>
</div></div>