[llvm-branch-commits] [llvm] edc57e7 - Guard `find_library(tensorflow_c_api ...)` by checking for TENSORFLOW_C_LIB_PATH to be set by the user

[Tom Stellard via llvm-branch-commits]

Author: Mehdi Amini
Date: 2020-12-08T23:12:26-05:00
New Revision: edc57e7e7ca2769d4d63bc939396f0d60666d262

URL: https://github.com/llvm/llvm-project/commit/edc57e7e7ca2769d4d63bc939396f0d60666d262
DIFF: https://github.com/llvm/llvm-project/commit/edc57e7e7ca2769d4d63bc939396f0d60666d262.diff

LOG: Guard `find_library(tensorflow_c_api ...)` by checking for TENSORFLOW_C_LIB_PATH to be set by the user

Also have CMake fails if the user provides a TENSORFLOW_C_LIB_PATH but
we can't find TensorFlow at this path.

At the moment the CMake script tries to figure if TensorFlow is
available on the system and enables support for it. This is in general
not desirable to customize build features this way and instead it is
preferable to let the user opt-in explicitly into the features they want
to enable. This is in line with other optional external dependencies
like Z3.
There are a few reasons to this but amongst others:
- reproducibility: making features "magically" enabled based on whether
  we find a package on the system or not makes it harder to handle bug
  reports from users.
- user control: they can't have TensorFlow on the system and build LLVM
  without TensorFlow right now. They also would suddenly distribute LLVM
  with a different set of features unknowingly just because their build
  machine environment would change subtly.

Right now this is motivated by a user reporting build failures on their system:

.../mesa-git/llvm-git/src/llvm-project/llvm/lib/Analysis/TFUtils.cpp:23:10: fatal error: tensorflow/c/c_api.h: No such file or directory
   23 | #include "tensorflow/c/c_api.h"
      |          ^~~~~~

It looks like we detected TensorFlow at configure time but couldn't set all the paths correctly.

Differential Revision: https://reviews.llvm.org/D88371

(cherry picked from commit e72d792c147ee506e337401e20c0f23042cc43fe)

Added: 
    

Modified: 
    llvm/CMakeLists.txt

Removed: 
    


################################################################################
diff  --git a/llvm/CMakeLists.txt b/llvm/CMakeLists.txt
index 915400af7a83..b8dabbbca05a 100644
--- a/llvm/CMakeLists.txt
+++ b/llvm/CMakeLists.txt
@@ -832,6 +832,11 @@ configure_file(
   ${LLVM_INCLUDE_DIR}/llvm/Config/Targets.def
   )
 
+# They are not referenced. See set_output_directory().
+set( CMAKE_RUNTIME_OUTPUT_DIRECTORY ${LLVM_BINARY_DIR}/bin )
+set( CMAKE_LIBRARY_OUTPUT_DIRECTORY ${LLVM_BINARY_DIR}/lib${LLVM_LIBDIR_SUFFIX} )
+set( CMAKE_ARCHIVE_OUTPUT_DIRECTORY ${LLVM_BINARY_DIR}/lib${LLVM_LIBDIR_SUFFIX} )
+
 # For up-to-date instructions for installing the Tensorflow dependency, refer to
 # the bot setup script: https://github.com/google/ml-compiler-opt/blob/master/buildbot/buildbot_init.sh
 # In this case, the latest C API library is available for download from
@@ -840,9 +845,9 @@ configure_file(
 # LLVM_HAVE_TF_API, through llvm-config.h, so that a user of the LLVM library may
 # also leverage the dependency.
 set(TENSORFLOW_C_LIB_PATH "" CACHE PATH "Path to TensorFlow C library install")
-find_library(tensorflow_c_api tensorflow PATHS ${TENSORFLOW_C_LIB_PATH}/lib)
 
-if (tensorflow_c_api)
+if (TENSORFLOW_C_LIB_PATH)
+  find_library(tensorflow_c_api tensorflow PATHS ${TENSORFLOW_C_LIB_PATH}/lib NO_DEFAULT_PATH REQUIRED)
   set(LLVM_HAVE_TF_API "ON" CACHE BOOL "Full Tensorflow API available")
   include_directories(${TENSORFLOW_C_LIB_PATH}/include)
 endif()
@@ -877,12 +882,6 @@ add_custom_target(srpm
   COMMAND rpmbuild -bs --define '_topdir ${LLVM_SRPM_DIR}' ${LLVM_SRPM_BINARY_SPECFILE})
 set_target_properties(srpm PROPERTIES FOLDER "Misc")
 
-
-# They are not referenced. See set_output_directory().
-set( CMAKE_RUNTIME_OUTPUT_DIRECTORY ${LLVM_BINARY_DIR}/bin )
-set( CMAKE_LIBRARY_OUTPUT_DIRECTORY ${LLVM_BINARY_DIR}/lib${LLVM_LIBDIR_SUFFIX} )
-set( CMAKE_ARCHIVE_OUTPUT_DIRECTORY ${LLVM_BINARY_DIR}/lib${LLVM_LIBDIR_SUFFIX} )
-
 if(APPLE AND DARWIN_LTO_LIBRARY)
   set(CMAKE_EXE_LINKER_FLAGS
     "${CMAKE_EXE_LINKER_FLAGS} -Wl,-lto_library -Wl,${DARWIN_LTO_LIBRARY}")