[PATCH] D43213: [Fuzzer] Avoid the unnecessary rebuild of the custom libc++

[Vitaly Buka via Phabricator via llvm-commits]

vitalybuka added inline comments.


================
Comment at: compiler-rt/cmake/Modules/AddCompilerRT.cmake:530
                ${sysroot_arg}
-               -DCMAKE_C_FLAGS=${LIBCXX_CFLAGS}
-               -DCMAKE_CXX_FLAGS=${LIBCXX_CFLAGS}
+               -DCMAKE_C_FLAGS=${LIBCXX_C_FLAGS}
+               -DCMAKE_CXX_FLAGS=${LIBCXX_CXX_FLAGS}
----------------
Can you please add into patch description explanation when it is expected to be rebuilt


================
Comment at: compiler-rt/cmake/Modules/AddCompilerRT.cmake:539
+    STEP_TARGETS configure build
+    BUILD_ALWAYS 1
+    USES_TERMINAL_CONFIGURE 1
----------------
Have you considered to achieve the same with just:
BUILD_ALWAYS 0
BUILD_BYPRODUCTS <libs>


================
Comment at: compiler-rt/cmake/Modules/AddCompilerRT.cmake:541
+    USES_TERMINAL_CONFIGURE 1
+    USES_TERMINAL_BUILD 1
+    USES_TERMINAL_INSTALL 1
----------------
Why do you need USES_TERMINAL_ ?


================
Comment at: compiler-rt/lib/fuzzer/CMakeLists.txt:89
     add_custom_command(TARGET clang_rt.${name}-${arch} POST_BUILD
-      COMMAND ${CMAKE_LINKER} --whole-archive "$<TARGET_LINKER_FILE:clang_rt.${name}-${arch}>" --no-whole-archive ${dir}/src/libcxx_fuzzer_${arch}-build/lib/libc++.a -r -o ${name}.o
+      COMMAND ${CMAKE_LINKER} --whole-archive "$<TARGET_LINKER_FILE:clang_rt.${name}-${arch}>" --no-whole-archive ${dir}-bins/lib/libc++.a -r -o ${name}.o
       COMMAND ${CMAKE_OBJCOPY} --localize-hidden ${name}.o
----------------
Can this be just ${dir}/src/libcxx_fuzzer_${arch}/lib/libc++.a ?


================
Comment at: compiler-rt/test/tsan/lit.cfg:59
   libcxx_path = os.path.join(config.compiler_rt_obj_root, "lib",
-                             "tsan", "libcxx_tsan_" + config.target_arch)
-  libcxx_incdir = os.path.join(libcxx_path, "include", "c++", "v1")
+                             "tsan", "libcxx_tsan_%s-bins" % config.target_arch)
+  libcxx_incdir = os.path.join(config.libcxx_path, "include")
----------------
why do we need "-bins" suffix?


Repository:
  rCRT Compiler Runtime

https://reviews.llvm.org/D43213