beanz added inline comments.
================
Comment at: runtime/CMakeLists.txt:41
@@ +40,3 @@
+
+ add_custom_target(compiler-rt-clear
+ DEPENDS ${CMAKE_CURRENT_BINARY_DIR}/compiler-rt-cleared
----------------
samsonov wrote:
> So, that's the target that you can invoke manually to clean compiler-rt build?
Yes.
================
Comment at: runtime/CMakeLists.txt:46
@@ +45,3 @@
+ OUTPUT ${CMAKE_CURRENT_BINARY_DIR}/compiler-rt-cleared
+ DEPENDS clang llvm-config
+ COMMAND ${CMAKE_COMMAND} -E remove_directory ${BINARY_DIR}
----------------
samsonov wrote:
> Hm? But "compiler-rt" target also depends on clang and llvm-config.
It should. We need it to rebuild whenever clang and llvm-config change.
Although it just occurred to me that I also need to make sure that the clearing
happens before compiler-rt gets built, so there will need to be a dependency on
that too... I'll figure that out.
================
Comment at: runtime/CMakeLists.txt:69
@@ -47,3 +68,3 @@
-DCOMPILER_RT_INCLUDE_TESTS=${LLVM_INCLUDE_TESTS}
- -DCOMPILER_RT_ENABLE_WERROR=ON
+ -DCMAKE_INSTALL_PREFIX=${CMAKE_INSTALL_PREFIX}
INSTALL_COMMAND ""
----------------
samsonov wrote:
> I thought we only refer to COMPILER_RT_INSTALL_PATH in compiler-rt's CMake.
> Do we actually need CMAKE_INSTALL_PREFIX there as well?
I don't think we're actually using it in compiler-rt today, but I think that
passing it through is a good idea so that if in the future compiler-rt ever
uses CMAKE_INSTALL_PREFIX it will be properly populated.
================
Comment at: runtime/CMakeLists.txt:70
@@ -49,2 +69,3 @@
+ -DCMAKE_INSTALL_PREFIX=${CMAKE_INSTALL_PREFIX}
INSTALL_COMMAND ""
STEP_TARGETS configure build
----------------
samsonov wrote:
> Disagree - we can't set COMPILER_RT_ENABLE_WERROR in compiler-rt's CMake,
> because standalone compiler-rt can be built with any compiler in the world,
> which we don't control. OTOH, if we *know* we're building it with trunk
> Clang, having it -Werror-clean is smth. we can enforce. It's also consistent
> with what autoconf did previously.
At the very least if we do that it should be tied to LLVM_ENABLE_WERROR, not
just defaulted to On. None of our projects default WERROR to On anywhere. While
I agree building with Werror is desirable, we shouldn't be defaulting it on,
and we certainly shouldn't be doing it in a way that makes it so the user has
to edit the build files in order to turn it off.
================
Comment at: runtime/CMakeLists.txt:78
@@ -62,11 +77,3 @@
- ExternalProject_Add_Step(compiler-rt clobber
- COMMAND ${CMAKE_COMMAND} -E remove_directory <BINARY_DIR>
- COMMAND ${CMAKE_COMMAND} -E make_directory <BINARY_DIR>
- COMMENT "Clobberring compiler-rt build directory..."
- DEPENDERS configure
- DEPENDS ${LLVM_RUNTIME_OUTPUT_INTDIR}/clang
- )
-
- ExternalProject_Get_Property(compiler-rt BINARY_DIR)
- set(COMPILER_RT_BINARY_DIR ${BINARY_DIR})
+ add_custom_target(install-compiler-rt
+ DEPENDS compiler-rt
----------------
samsonov wrote:
> This is also convenience target, right?
Yes.
http://reviews.llvm.org/D13399
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
