[PATCH] D15462: [CMake] Add support for generating profdata for clang from training files

[Vedant Kumar via cfe-commits]

vsk added a comment.

Thanks! Interesting approach. I think @cmatthews would appreciate a `generate-profdata-from-lnt` target if you're up for it :).

Comments inline --


================
Comment at: utils/perf-training/CMakeLists.txt:2
@@ +1,3 @@
+if(LLVM_BUILD_INSTRUMENTED AND NOT WIN32)
+  if (CMAKE_CFG_INTDIR STREQUAL ".")
+    set(LLVM_BUILD_MODE ".")
----------------
Afaict, lines 2-8 look like they're provided by configure_lit_site_cfg (AddLLVM.cmake).

================
Comment at: utils/perf-training/CMakeLists.txt:8
@@ +7,3 @@
+
+  string(REPLACE ${CMAKE_CFG_INTDIR} ${LLVM_BUILD_MODE} CLANG_TOOLS_DIR ${LLVM_RUNTIME_OUTPUT_INTDIR})
+
----------------
Is it possible for this line to interact poorly with the one in clang/test/CMakeLists.txt? E.g something weird like if CMAKE_CFG_INTDIR is a substring of LLVM_BUILD_MODE?

================
Comment at: utils/perf-training/CMakeLists.txt:17
@@ +16,3 @@
+    ${CMAKE_CURRENT_BINARY_DIR}
+    DEPENDS clang clear-profraw
+    )
----------------
Can we build llvm-profdata too?

================
Comment at: utils/perf-training/CMakeLists.txt:25
@@ +24,3 @@
+  if(NOT LLVM_PROFDATA)
+    find_program(LLVM_PROFDATA llvm-profdata)
+  endif()
----------------
We should use a llvm-profdata built out of the current source. Does the `llvm_find_program` function help with this?

================
Comment at: utils/perf-training/CMakeLists.txt:33
@@ +32,3 @@
+  add_custom_target(generate-profdata
+    COMMAND find ${CMAKE_CURRENT_BINARY_DIR} -name "*.profraw" -print | xargs ${LLVM_PROFDATA} merge -o ${CMAKE_CURRENT_BINARY_DIR}/clang.profdata
+    COMMENT "Merging profdata"
----------------
Sean's comment about relying on `find` applies here too.

================
Comment at: utils/perf-training/lit.cfg:7
@@ +6,3 @@
+
+def inferClang(PATH):
+    # Determine which clang to use.
----------------
This duplicates code in tools/clang/test/lit.cfg. Can we lift it into a shared module?

================
Comment at: utils/perf-training/lit.site.cfg.in:3
@@ +2,3 @@
+
+## Autogenerated by LLVM/Clang configuration.
+# Do not edit!
----------------
I don't know too much about what's going on here. It seems weird to check in auto generated files.. why do we need to do that?


http://reviews.llvm.org/D15462