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

[Petr Hosek via Phabricator via llvm-commits]

phosek added inline comments.


================
Comment at: compiler-rt/cmake/Modules/AddCompilerRT.cmake:539
+    STEP_TARGETS configure build
+    BUILD_ALWAYS 1
+    USES_TERMINAL_CONFIGURE 1
----------------
vitalybuka wrote:
> Have you considered to achieve the same with just:
> BUILD_ALWAYS 0
> BUILD_BYPRODUCTS <libs>
The problem is that we would need to list all the library artifacts, e.g. `libc++.so` is only a symlink to `libc++.so.1` which is a symlink to `libc++.so.1.0`, and I don't know if we want to rely on those details here?


================
Comment at: compiler-rt/cmake/Modules/AddCompilerRT.cmake:541
+    USES_TERMINAL_CONFIGURE 1
+    USES_TERMINAL_BUILD 1
+    USES_TERMINAL_INSTALL 1
----------------
vitalybuka wrote:
> Why do you need USES_TERMINAL_ ?
This is not really needed, but now that libc++ is being used as part of the libFuzzer build, I just found it much easier to debug build issues if the libFuzzer's libc++ build output goes to terminal rather than file log which requires extra steps to read, I'm happy to drop this if you don't like it though.


================
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
----------------
vitalybuka wrote:
> Can this be just ${dir}/src/libcxx_fuzzer_${arch}/lib/libc++.a ?
That directory no longer exists, all the binary artifacts now end up in the -bins directory.


================
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")
----------------
vitalybuka wrote:
> why do we need "-bins" suffix?
I was following the naming convention used by llvm_ExternalProject_Add, but I can drop the "-bins" suffix if you prefer?


Repository:
  rCRT Compiler Runtime

https://reviews.llvm.org/D43213