[cmake] Move LLVM_TOUCH_STATIC_LIBRARIES logic to llvm_update_compile_flags

[hintonda]

Summary

This is a refinement of D62172, r361280, and moves the
LLVM_TOUCH_STATIC_LIBRARIES logic to llvm_update_compile_flags so it
can be used by static libraries created with add_library and not just
those created with llvm_add_library.

[hintonda]

ping...

[hintonda]

@beanz, could you take a look? thanks...

[beanz]

I don't like the idea of putting this in llvm_update_compile_flags. That function does exactly what its name says, and adding additional behavior to it seems undesirable. Where are we creating libraries with add_library that we need this?

One thought would be to make a new CMake function to apply the workaround.

[hintonda]

In D62455#1574256, @beanz wrote:

I don't like the idea of putting this in llvm_update_compile_flags. That function does exactly what its name says, and adding additional behavior to it seems undesirable. Where are we creating libraries with add_library that we need this?

There might be others, but these are the ones I always see:

$ find llvm -type f \( -name "*.cmake" -o -name "*.txt" \) -exec grep -Hn llvm_update_compile_flags \{\} \;
llvm/tools/llvm-cfi-verify/lib/CMakeLists.txt:9:llvm_update_compile_flags(LLVMCFIVerify)
llvm/tools/llvm-exegesis/lib/CMakeLists.txt:38:llvm_update_compile_flags(LLVMExegesis)
llvm/tools/llvm-exegesis/lib/PowerPC/CMakeLists.txt:11:llvm_update_compile_flags(LLVMExegesisPowerPC)
llvm/tools/llvm-exegesis/lib/X86/CMakeLists.txt:11:llvm_update_compile_flags(LLVMExegesisX86)
llvm/tools/llvm-exegesis/lib/AArch64/CMakeLists.txt:11:llvm_update_compile_flags(LLVMExegesisAArch64)

One thought would be to make a new CMake function to apply the workaround.

Okay, I'll see if I can come up with something as clean as this.

[beanz]

One thought, we should probably evaluate why those tools are using add_library instead of add_llvm_library. The right fix might be to move them over. This is exactly the kind of behavior that add_llvm_library and llvm_add_library were designed to handle.

[hintonda]

In D62455#1576306, @beanz wrote:

One thought, we should probably evaluate why those tools are using add_library instead of add_llvm_library. The right fix might be to move them over. This is exactly the kind of behavior that add_llvm_library and llvm_add_library were designed to handle.

I agree, however, I remember a lot of churn when these were added, and didn't really follow why they needed to be different, so I was hesitant to make any gratuitous changes. My goal was just to have nice clean builds on my laptop that didn't take too long or report spurious errors.

I'll take another look and see if I can figure out why they didn't work out of the box.

llvm/cmake/modules/AddLLVM.cmake

 include(LLVMProcessSources)
 include(LLVM-Config)
 include(DetermineGCCCompatible)
 
 function(llvm_update_compile_flags name)
+  # clang and newer versions of ninja use high-resolutions timestamps,
+  # but older versions of libtool on Darwin don't, so the archive will
+  # often get an older timestamp than the last object that was added
+  # or updated.  To fix this, we add a custom command to touch archive
+  # after it's been built so that ninja won't rebuild it unnecessarily
+  # the next time it's run.
+  if(LLVM_TOUCH_STATIC_LIBRARIES)
+    get_target_property(target_type ${name} TYPE)
+    if (target_type STREQUAL "STATIC_LIBRARY")
+      add_custom_command(TARGET ${name}
+        POST_BUILD
+        COMMAND touch ${LLVM_LIBRARY_DIR}/${CMAKE_STATIC_LIBRARY_PREFIX}${name}${CMAKE_STATIC_LIBRARY_SUFFIX}
+        )
+    endif ()
+  endif()
+
   get_property(sources TARGET ${name} PROPERTY SOURCES)
   if("${sources}" MATCHES "\\.c(;|$)")
     set(update_src_props ON)
   endif()
 
   # LLVM_REQUIRES_EH is an internal flag that individual targets can use to
   # force EH
   if(LLVM_REQUIRES_EH OR LLVM_ENABLE_EH)
foreach(objlib ${objlibs})
       add_dependencies(${objlib} ${LLVM_COMMON_DEPENDS})
     endforeach()
   endif()
 
   if(ARG_SHARED OR ARG_MODULE)
     llvm_externalize_debuginfo(${name})
     llvm_codesign(${name} ENTITLEMENTS ${ARG_ENTITLEMENTS})
   endif()
-  # clang and newer versions of ninja use high-resolutions timestamps,
-  # but older versions of libtool on Darwin don't, so the archive will
-  # often get an older timestamp than the last object that was added
-  # or updated.  To fix this, we add a custom command to touch archive
-  # after it's been built so that ninja won't rebuild it unnecessarily
-  # the next time it's run.
-  if(ARG_STATIC AND LLVM_TOUCH_STATIC_LIBRARIES)
-    add_custom_command(TARGET ${name}
-      POST_BUILD
-      COMMAND touch ${LLVM_LIBRARY_DIR}/${CMAKE_STATIC_LIBRARY_PREFIX}${name}${CMAKE_STATIC_LIBRARY_SUFFIX}
-      )
-  endif()
 endfunction()
 
 function(add_llvm_install_targets target)
   cmake_parse_arguments(ARG "" "COMPONENT;PREFIX" "DEPENDS" ${ARGN})
   if(ARG_COMPONENT)
     set(component_option -DCMAKE_INSTALL_COMPONENT="${ARG_COMPONENT}")
   endif()
   if(ARG_PREFIX)