Chris Bieneman <be...@apple.com> writes: > beanz created this revision. > beanz added a reviewer: bogner. > beanz added a subscriber: cfe-commits. > > This commit re-lands r259862. The underlying cause of the build > breakage was an incorrectly written capabilities test. In > tools/Driver/CMakeLists.txt I was attempting to check if a linker flag > worked, the test was passing it to the compiler, not the linker. CMake > doesn't have a linker test, so we have a hand-rolled one. > > Original Patch Review: http://reviews.llvm.org/D16896 > > Original Summary: > With this change generating clang order files using dtrace uses the > following workflow: > > cmake <whatever options you want> > > ninja generate-order-file > > ninja clang > > This patch works by setting a default path to the order file (which > can be overridden by the user). If the order file doesn't exist during > configuration CMake will create an empty one. > > CMake then ties up the dependencies between the clang link job and the > order file, and generate-order-file overwrites CLANG_ORDER_FILE with > the new order file.
This looks it'll work. I do have one comment / concern below, but as long as that's addressed this LGTM. > http://reviews.llvm.org/D16999 > > Files: > CMakeLists.txt > tools/driver/CMakeLists.txt > utils/perf-training/CMakeLists.txt > > Index: utils/perf-training/CMakeLists.txt > =================================================================== > --- utils/perf-training/CMakeLists.txt > +++ utils/perf-training/CMakeLists.txt > @@ -55,9 +55,8 @@ > COMMAND ${PYTHON_EXECUTABLE} ${CMAKE_CURRENT_SOURCE_DIR}/perf-helper.py > clean ${CMAKE_CURRENT_BINARY_DIR} dtrace > COMMENT "Clearing old dtrace data") > > - > add_custom_target(generate-order-file > - COMMAND ${PYTHON_EXECUTABLE} ${CMAKE_CURRENT_SOURCE_DIR}/perf-helper.py > gen-order-file --binary $<TARGET_FILE:clang> --output > ${CMAKE_CURRENT_BINARY_DIR}/clang.order ${CMAKE_CURRENT_BINARY_DIR} > + COMMAND ${PYTHON_EXECUTABLE} ${CMAKE_CURRENT_SOURCE_DIR}/perf-helper.py > gen-order-file --binary $<TARGET_FILE:clang> --output ${CLANG_ORDER_FILE} > ${CMAKE_CURRENT_BINARY_DIR} > COMMENT "Generating order file" > DEPENDS generate-dtrace-logs) > endif() > Index: tools/driver/CMakeLists.txt > =================================================================== > --- tools/driver/CMakeLists.txt > +++ tools/driver/CMakeLists.txt > @@ -87,8 +87,24 @@ > set(TOOL_INFO_BUILD_VERSION) > endif() > > -if(CLANG_ORDER_FILE AND EXISTS CLANG_ORDER_FILE) > - target_link_libraries(clang "-Wl,-order_file,${CLANG_ORDER_FILE}") > +if(CLANG_ORDER_FILE) > + include(CMakePushCheckState) > + > + function(check_linker_flag flag out_var) > + cmake_push_check_state() > + set(CMAKE_REQUIRED_FLAGS "${CMAKE_REQUIRED_FLAGS} ${flag}") > + check_cxx_compiler_flag("" ${out_var}) > + cmake_pop_check_state() > + endfunction() > + > + # This is a test to ensure the actual order file works with the linker. > + check_linker_flag("-Wl,-order_file,${CLANG_ORDER_FILE}" > + LINKER_ORDER_FILE_WORKS) > + > + if(LINKER_ORDER_FILE_WORKS) > + target_link_libraries(clang "-Wl,-order_file,${CLANG_ORDER_FILE}") > + set_target_properties(clang PROPERTIES LINK_DEPENDS ${CLANG_ORDER_FILE}) > + endif() > endif() > > if(WITH_POLLY AND LINK_POLLY_INTO_TOOLS) > Index: CMakeLists.txt > =================================================================== > --- CMakeLists.txt > +++ CMakeLists.txt > @@ -586,18 +586,19 @@ > add_subdirectory(docs) > endif() > > -if(EXISTS "${CMAKE_CURRENT_BINARY_DIR}/clang.order") > - file(REMOVE "${CMAKE_CURRENT_BINARY_DIR}/clang.order") > -endif() > - > -if(CLANG_ORDER_FILE STREQUAL "${CMAKE_CURRENT_BINARY_DIR}/clang.order") > +# this line is needed as a cleanup to ensure that any CMakeCaches with the > old > +# default value get updated to the new default. > +if(CLANG_ORDER_FILE STREQUAL "") > unset(CLANG_ORDER_FILE CACHE) > - unset(CLANG_ORDER_FILE) > endif() > > -set(CLANG_ORDER_FILE "" CACHE FILEPATH > +set(CLANG_ORDER_FILE ${CMAKE_CURRENT_BINARY_DIR}/clang.order CACHE FILEPATH > "Order file to use when compiling clang in order to improve startup time.") > > +if(CLANG_ORDER_FILE AND NOT EXISTS ${CLANG_ORDER_FILE}) > + file(WRITE ${CLANG_ORDER_FILE} "\n") > +endif() What happens if someone typos their order file? Won't we end up writing an empty file somewhere random on their filesystem? I think we should only do the "create an empty file" thing if the order file is inside ${CMAKE_CURRENT_BINARY_DIR} - if someone specifies -DCLANG_ORDER_FILE=/path/to/nonexistent they should get an error. > + > if (CLANG_BUILT_STANDALONE OR CMAKE_VERSION VERSION_EQUAL 3 OR > CMAKE_VERSION VERSION_GREATER 3) > # Generate a list of CMake library targets so that other CMake projects can > _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
