Generate files per configuration & change build output directories (l…

…lvm#616)

* Generate files per configuration & refer to them as such

This is a step towards fully supporting multi-config generators such as VS and Xcode. Right now, the generated files as well as path to files in the project point to a single location which often does not exist unless CMAKE_BUILD_TYPE was specified on the command line which defeats the purpose of multi-config.

This change does a few things:
1) Files that are generated by the build now reside in the config directory when using a multi-config generator. For example, instead of: src/ExternalUtil.hpp, when using VS or Xcode, we have: src/Debug/ExternalUtil.hpp, etc.
2) The targets that use the generated files now have an explicit dependency
3) The lit configuration now correctly refers to the binaries in the onnx-mlir build tree per configuration
4) The paths to onnx-mlir and the build directories are correct in the generated files
5) Executables now have the right extensions when they are referenced in tools and scripts
6) A couple of unnecessary variables are removed

Bonus: move the CTest includes where they are needed

After this change, the path to LLVM_PROJ_BIN is still not correct always since it still refers to CMAKE_BUILD_TYPE which is not always known at configuration time. This can be fixed by using find_package(LLVM) or find_package(MLIR). Using find_package is optimal, but it is a bigger change, so I am splitting it from the file generation for simplicity. The find_package(MLIR) change will be sent for review after this change (and maybe a couple more simple ones) is in.

Signed-off-by: Stella Stamenova <stilis@microsoft.com>

* Correctly generate the files for single generators as well

Signed-off-by: Stella Stamenova <stilis@microsoft.com>

* Unfortunately, generator expressions are not enough to make this work
and we have to do it the hard way

Signed-off-by: Stella Stamenova <stilis@microsoft.com>

* Fix a copy-paste error

Signed-off-by: Stella Stamenova <stilis@microsoft.com>

* Fix another copy-paste error because apparently I need to learn how to copy-paste

Signed-off-by: Stella Stamenova <stilis@microsoft.com>

* Make all python string raw strings - on (windows) systems where the path contains short directories such as a, \a ends up being treated as a special character causing failures

Signed-off-by: Stella Stamenova <stilis@microsoft.com>

* Add comments to explain the usage of CMAKE_CFG_INTDIR and add it for lit as well for consistency

Signed-off-by: Stella Stamenova <stilis@microsoft.com>

* Don't move the ctest include

It is not pertinent to the multiconfig changes and it is causing a weird issue with test generation on some platforms

Signed-off-by: Stella Stamenova <stilis@microsoft.com>

* After thinking through the problem some more, we can simplify the logic for the generation of the files by using generator expressions in a couple of more places including the CMAKE_*_OUTPUT_DIRECTORY properties which support generator expressions. This allows the files to be generated for each config option and then we can always reference them in that location by using CMAKE_BUILD_TYPE when dealing with a single-config generator.

By making this change this way, the CMAKE_*_OUTPUT_DIRECTORY set now fully controls where all of the outputs are located which means that if we wanted to align with LLVM for the binary layout, it is a simple 3-line change.

Signed-off-by: Stella Stamenova <stilis@microsoft.com>

* Add comment in runtime/jni/cmakelists.txt to explain why we need to copy the file.

Signed-off-by: Stella Stamenova <stilis@microsoft.com>

* Set the build path for both single- and multi-config generators to contain the build mode. For example, build_dir/bin/release and build_dir/lib_release

Signed-off-by: Stella Stamenova <stilis@microsoft.com>

* Change the build paths for onnx-mlir from build_dir/bin/release to build_dir/release/bin

Signed-off-by: Stella Stamenova <stilis@microsoft.com>

Co-authored-by: Kevin O'Brien <caomhin@us.ibm.com>

CMakeLists.txt

@@ -12,12 +12,17 @@ endif()
 
 set(CMAKE_CXX_FLAGS_RELEASE "-O2 -DNDEBUG")
 
-set(ONNX_MLIR_SRC_ROOT "${CMAKE_CURRENT_SOURCE_DIR}")
-set(ONNX_MLIR_BIN_ROOT "${CMAKE_CURRENT_BINARY_DIR}")
+set(CMAKE_ARCHIVE_OUTPUT_DIRECTORY ${CMAKE_BINARY_DIR}/$<CONFIG>/lib)
+set(CMAKE_LIBRARY_OUTPUT_DIRECTORY ${CMAKE_BINARY_DIR}/$<CONFIG>/lib)
+set(CMAKE_RUNTIME_OUTPUT_DIRECTORY ${CMAKE_BINARY_DIR}/$<CONFIG>/bin)
 
-set(CMAKE_ARCHIVE_OUTPUT_DIRECTORY ${CMAKE_BINARY_DIR}/lib)
-set(CMAKE_LIBRARY_OUTPUT_DIRECTORY ${CMAKE_BINARY_DIR}/lib)
-set(CMAKE_RUNTIME_OUTPUT_DIRECTORY ${CMAKE_BINARY_DIR}/bin)
+set(ONNX_MLIR_SRC_ROOT ${CMAKE_CURRENT_SOURCE_DIR})
+set(ONNX_MLIR_BIN_ROOT ${CMAKE_CURRENT_BINARY_DIR})
+
+# This is the opposite of LLVM which places CMAKE_CFG_INTDIR first, but to do
+# that it does a bunch of heavy lifting in the CMake files.
+set(ONNX_MLIR_LIBRARY_PATH ${CMAKE_LIBRARY_OUTPUT_DIRECTORY})
+set(ONNX_MLIR_RUNTIME_PATH ${CMAKE_RUNTIME_OUTPUT_DIRECTORY})
 
 # Lit test suite requires at least python 3.6
 set(LLVM_MINIMUM_PYTHON_VERSION 3.6)

MLIR.cmake

@@ -73,21 +73,6 @@ endif()
 set(LLVM_BIN_INCLUDE_PATH ${LLVM_PROJ_BUILD}/include)
 set(MLIR_SRC_INCLUDE_PATH ${LLVM_PROJ_SRC}/mlir/include)
 set(MLIR_BIN_INCLUDE_PATH ${LLVM_PROJ_BUILD}/tools/mlir/include)
-set(MLIR_TOOLS_DIR ${LLVM_PROJ_BIN})
-
-# ONNX-MLIR tools folder
-if(MSVC)
-  if (CMAKE_BUILD_TYPE)
-    set(ONNX_MLIR_TOOLS_DIR ${CMAKE_BINARY_DIR}/bin/${CMAKE_BUILD_TYPE})
-  else()
-    set(ONNX_MLIR_TOOLS_DIR ${CMAKE_BINARY_DIR}/bin/Release)
-  endif()
-else()
-  set(ONNX_MLIR_TOOLS_DIR ${CMAKE_BINARY_DIR}/bin)
-endif()
-message(STATUS "ONNX_MLIR_TOOLS_DIR     : " ${ONNX_MLIR_TOOLS_DIR})
-set(ONNX_MLIR_LIT_TEST_SRC_DIR ${ONNX_MLIR_SRC_ROOT}/test/mlir)
-set(ONNX_MLIR_LIT_TEST_BUILD_DIR ${CMAKE_BINARY_DIR}/test/mlir)
 
 set(
         MLIR_INCLUDE_PATHS
@@ -119,17 +104,6 @@ if(NOT MSVC)
   set(MLIR_SYSTEM_LIBS ${MLIR_SYSTEM_LIBS} ${ZLIB_LIBRARIES} ${CURSES_LIBRARIES})
 endif()
 
-# Set output library path
-if(MSVC)
-  if (CMAKE_BUILD_TYPE)
-    set(ONNX_MLIR_LIB_DIR ${CMAKE_LIBRARY_OUTPUT_DIRECTORY}/${CMAKE_BUILD_TYPE})
-  else()
-    set(ONNX_MLIR_LIB_DIR ${CMAKE_LIBRARY_OUTPUT_DIRECTORY}/Release)
-  endif()
-else()
-    set(ONNX_MLIR_LIB_DIR ${CMAKE_LIBRARY_OUTPUT_DIRECTORY})
-endif()
-
 function(find_mlir_lib lib)
   find_library(${lib}
           NAMES ${lib}

src/CMakeLists.txt

@@ -10,61 +10,81 @@ add_subdirectory(Runtime)
 
 # All ONNX-MLIR libraries.
 set(OMLibs
-        OMBuilder
-        # Note that circular dependencies exist between
-        # ONNXOps and KrnlOps, so we include them twice
-        # for full symbol resolution.
-        OMONNXOps
-        OMKrnlOps
-        OMONNXOps
-        OMKrnlOps
-        OMSupport
-        OMKrnlToAffine
-        OMKrnlToLLVM
-        OMONNXToKrnl
-        OMONNXRewrite
-        OMShapeInference
-        OMShapeInferenceOpInterface
-        OMResultTypeInferenceOpInterface
-        OMHasOnnxSubgraphOpInterface
-        OMSpecializedKernelOpInterface
-        OMElideConstants
-        OMElideKrnlGlobalConstants
-        OMEnableMemoryPool
-        OMBundleMemoryPools
-        OMOptimizeMemoryPools
-        OMDisconnectKrnlDimFromAlloc
-        OMLowerKrnlShape
-        OMSimplifyKrnl)
+  OMBuilder
+  # Note that circular dependencies exist between
+  # ONNXOps and KrnlOps, so we include them twice
+  # for full symbol resolution.
+  OMONNXOps
+  OMKrnlOps
+  OMONNXOps
+  OMKrnlOps
+  OMSupport
+  OMKrnlToAffine
+  OMKrnlToLLVM
+  OMONNXToKrnl
+  OMONNXRewrite
+  OMShapeInference
+  OMShapeInferenceOpInterface
+  OMResultTypeInferenceOpInterface
+  OMHasOnnxSubgraphOpInterface
+  OMSpecializedKernelOpInterface
+  OMElideConstants
+  OMElideKrnlGlobalConstants
+  OMEnableMemoryPool
+  OMBundleMemoryPools
+  OMOptimizeMemoryPools
+  OMDisconnectKrnlDimFromAlloc
+  OMLowerKrnlShape
+  OMSimplifyKrnl
+  )
 set(OMLibs ${OMLibs} PARENT_SCOPE)
 
 add_subdirectory(Tool)
 
 add_library(MainUtils
-        MainUtils.hpp
-        MainUtils.cpp)
+  MainUtils.hpp
+  MainUtils.cpp
+  )
+
 target_link_libraries(MainUtils
-        ${OMLibs}
-        ${MLIRLibs}
-        ${CMAKE_DL_LIBS}
-        onnx)
+  PUBLIC
+  ${OMLibs}
+  ${MLIRLibs}
+  ${CMAKE_DL_LIBS}
+  onnx
+  )
 
 target_include_directories(MainUtils PRIVATE ${ONNX_MLIR_SRC_ROOT})
-target_include_directories(MainUtils PRIVATE ${CMAKE_BINARY_DIR})
 target_include_directories(MainUtils PRIVATE ${ONNX_MLIR_BIN_ROOT})
+target_include_directories(MainUtils PRIVATE ${CMAKE_CURRENT_SOURCE_DIR})
 
 add_executable(onnx-mlir
         main.cpp)
 target_link_libraries(onnx-mlir MainUtils)
 
-# Locate llc, which is needed for translating LLVM bitcode
-# to object file.
-if(NOT EXISTS "${LLVM_PROJ_BUILD}/bin/llc")
-  message(ERROR "Cannot find llc.")
+configure_file(
+  ${CMAKE_CURRENT_SOURCE_DIR}/ExternalUtil.hpp.in
+  ${CMAKE_CURRENT_BINARY_DIR}/ExternalUtil.hpp.cfg
+  @ONLY
+  )
+
+file(GENERATE
+  OUTPUT ${CMAKE_CURRENT_BINARY_DIR}/$<CONFIG>/ExternalUtil.hpp
+  INPUT ${CMAKE_CURRENT_BINARY_DIR}/ExternalUtil.hpp.cfg
+  )
+
+# CMAKE_CFG_INTDIR is . for single-config generators such as make, and
+# it has a value (e.g. $(Configuration)) otherwise, so we can use it to
+# determine whether we are dealing with a multi-config generator.
+if (NOT "${CMAKE_CFG_INTDIR}" STREQUAL ".")
+  set(FILE_GENERATE_DIR ${CMAKE_CURRENT_BINARY_DIR}/${CMAKE_CFG_INTDIR})
+else()
+  set(FILE_GENERATE_DIR ${CMAKE_CURRENT_BINARY_DIR}/${CMAKE_BUILD_TYPE})
 endif()
 
-configure_file(${CMAKE_CURRENT_SOURCE_DIR}/ExternalUtil.hpp.in
-        ${CMAKE_CURRENT_BINARY_DIR}/ExternalUtil.hpp)
+add_custom_target(ExternalUtil DEPENDS ${FILE_GENERATE_DIR}/ExternalUtil.hpp)
+add_dependencies(MainUtils ExternalUtil)
+target_include_directories(MainUtils PRIVATE ${FILE_GENERATE_DIR})
 
 # Libraries specified on the target_link_libraries for the add_subdirectory
 # targets get added to the end of the list here. This creates two problems:
@@ -79,7 +99,6 @@ add_dependencies(onnx-mlir OMKrnlOpsInc OMONNXOpsInc)
 add_dependencies(onnx-mlir cruntime)
 
 target_include_directories(onnx-mlir PRIVATE ${ONNX_MLIR_SRC_ROOT})
-target_include_directories(onnx-mlir PRIVATE ${CMAKE_BINARY_DIR})
 target_include_directories(onnx-mlir PRIVATE ${ONNX_MLIR_BIN_ROOT})
 
 install(TARGETS onnx-mlir DESTINATION bin)

src/ExternalUtil.hpp.in

@@ -3,10 +3,10 @@
 #include <string>
 
 namespace onnx_mlir {
-std::string kExecPath = "@CMAKE_INSTALL_PREFIX@/bin/onnx-mlir"; /* fallback if not set by main */
+std::string kExecPath = "@CMAKE_INSTALL_PREFIX@/bin/$<TARGET_FILE_NAME:onnx-mlir>"; /* fallback if not set by main */
 const std::string kInstPath = "@CMAKE_INSTALL_PREFIX@";
-const std::string kOptPath = "@LLVM_PROJ_BUILD@/bin/opt";
-const std::string kLlcPath = "@LLVM_PROJ_BUILD@/bin/llc";
+const std::string kOptPath = "@LLVM_PROJ_BIN@/opt@CMAKE_EXECUTABLE_SUFFIX@";
+const std::string kLlcPath = "@LLVM_PROJ_BIN@/llc@CMAKE_EXECUTABLE_SUFFIX@";
 const std::string kCxxPath = "@CMAKE_CXX_COMPILER@";
 const std::string kLinkerPath = "@CMAKE_LINKER@";
 const std::string kObjCopyPath = "@CMAKE_OBJCOPY@";

src/MainUtils.cpp

@@ -26,8 +26,8 @@
 #include "llvm/Support/FileSystem.h"
 #include "llvm/Support/Program.h"
 
-#include "src/ExternalUtil.hpp"
-#include "src/MainUtils.hpp"
+#include "ExternalUtil.hpp"
+#include "MainUtils.hpp"
 
 #ifdef _WIN32
 #include <io.h>

src/Runtime/jni/CMakeLists.txt

@@ -11,9 +11,14 @@ if(Java_Development_FOUND AND JNI_FOUND)
   # Target for Java runtime jar
   add_jar(javaruntime
           src/com/ibm/onnxmlir/OMModel.java
-	  src/com/ibm/onnxmlir/OMTensorList.java
-          src/com/ibm/onnxmlir/OMTensor.java
-          OUTPUT_DIR ${CMAKE_LIBRARY_OUTPUT_DIRECTORY})
+          src/com/ibm/onnxmlir/OMTensorList.java
+          src/com/ibm/onnxmlir/OMTensor.java)
+
+  # ONNX_MLIR_LIBRARY_PATH is a generator expression which is not supported by add_jar as the output
+  # directory. Instead, we let add_jar place the jar file in the default location and copy it to the
+  # library path after it is built.
+  add_custom_command(TARGET javaruntime POST_BUILD
+    COMMAND ${CMAKE_COMMAND} -E copy_if_different $<TARGET_PROPERTY:javaruntime,JAR_FILE> ${ONNX_MLIR_LIBRARY_PATH})
 
   # Target for JNI runtime lib
   add_library(jniruntime STATIC

test/backend/CMakeLists.txt

@@ -1,15 +1,29 @@
 # SPDX-License-Identifier: Apache-2.0
 
-# Set TEST_DRIVER_BUILD_PATH to the directory of the test driver.
-# It is expected to have two sub-directories:
-# - 'bin' containing executable files, and
-# - 'lib' containing necessary libraries for runtime execution.
-set (TEST_DRIVER_BUILD_PATH ${CMAKE_BINARY_DIR})
-# Set TEST_DRIVER_COMMAND to the command used to compile models.
-# The command is expected to be found in the 'bin' directory of ${TEST_DRIVER_BUILD_PATH}
-set (TEST_DRIVER_COMMAND "onnx-mlir")
-configure_file(test.py test.py COPYONLY)
-configure_file(test_config.py.in test_config.py)
+file(GENERATE
+  OUTPUT ${CMAKE_CURRENT_BINARY_DIR}/$<CONFIG>/test.py
+  INPUT ${CMAKE_CURRENT_SOURCE_DIR}/test.py
+  )
+
+configure_file(
+  ${CMAKE_CURRENT_SOURCE_DIR}/test_config.py.in
+  ${CMAKE_CURRENT_BINARY_DIR}/test_config.py.cfg
+  @ONLY
+  )
+
+file(GENERATE
+  OUTPUT ${CMAKE_CURRENT_BINARY_DIR}/$<CONFIG>/test_config.py
+  INPUT ${CMAKE_CURRENT_BINARY_DIR}/test_config.py.cfg
+  )
+
+# CMAKE_CFG_INTDIR is . for single-config generators such as make, and
+# it has a value (e.g. $(Configuration)) otherwise, so we can use it to
+# determine whether we are dealing with a multi-config generator.
+if (NOT "${CMAKE_CFG_INTDIR}" STREQUAL ".")
+  set(FILE_GENERATE_DIR ${CMAKE_CURRENT_BINARY_DIR}/${CMAKE_CFG_INTDIR})
+else()
+  set(FILE_GENERATE_DIR ${CMAKE_CURRENT_BINARY_DIR}/${CMAKE_BUILD_TYPE})
+endif()
 
 # Detect pytest-xdist for parallel backend tests
 execute_process(
@@ -37,18 +51,29 @@ if(EXISTS ${LIBSTDCXX_LIB})
 endif()
 
 add_custom_target(check-onnx-backend
-        COMMAND ONNX_HOME=${CMAKE_CURRENT_BINARY_DIR} ${BACKEND_TEST_COMMAND}
-        ${BACKEND_TEST_ARGS} ${CMAKE_CURRENT_BINARY_DIR}/test.py)
+  COMMAND
+    ONNX_HOME=${CMAKE_CURRENT_BINARY_DIR} ${BACKEND_TEST_COMMAND}
+    ${BACKEND_TEST_ARGS} ${FILE_GENERATE_DIR}/test.py
+  DEPENDS
+    ${FILE_GENERATE_DIR}/test.py
+    ${FILE_GENERATE_DIR}/test_config.py
+   )
 
 add_custom_target(check-onnx-backend-dynamic
-        COMMAND TEST_DYNAMIC=true ${BACKEND_TEST_COMMAND}
-        ${BACKEND_TEST_ARGS} ${CMAKE_CURRENT_BINARY_DIR}/test.py)
+  COMMAND
+    TEST_DYNAMIC=true ${BACKEND_TEST_COMMAND}
+    ${BACKEND_TEST_ARGS} ${FILE_GENERATE_DIR}/test.py
+  DEPENDS
+    ${FILE_GENERATE_DIR}/test.py
+    ${FILE_GENERATE_DIR}/test_config.py
+  )
 
 add_custom_target(clean-onnx-backend
-        COMMAND ${CMAKE_COMMAND} -E remove
-        ${CMAKE_CURRENT_BINARY_DIR}/*.onnx
-        ${CMAKE_CURRENT_BINARY_DIR}/*.so
-        )
+  COMMAND
+    ${CMAKE_COMMAND} -E remove
+    ${CMAKE_CURRENT_BINARY_DIR}/*.onnx
+    ${CMAKE_CURRENT_BINARY_DIR}/*.so
+  )
 
 add_dependencies(check-onnx-backend onnx-mlir)
 add_dependencies(check-onnx-backend PyRuntime)

test/backend/test.py

@@ -53,13 +53,12 @@
 print("temporary results are in dir "+result_dir)
 
 CXX = test_config.CXX_PATH
-TEST_DRIVER = os.path.join(test_config.TEST_DRIVER_BUILD_PATH, "bin",
-                           test_config.TEST_DRIVER_COMMAND)
-LLC = os.path.join(test_config.LLVM_PROJ_BUILD_PATH, "bin/llc")
+LLC = test_config.LLC_PATH
+RUNTIME_DIR = test_config.TEST_DRIVER_RUNTIME_PATH
+TEST_DRIVER = test_config.TEST_DRIVER_PATH
 
 # Make lib folder under build directory visible in PYTHONPATH
 doc_check_base_dir = os.path.dirname(os.path.realpath(__file__))
-RUNTIME_DIR = os.path.join(test_config.TEST_DRIVER_BUILD_PATH, "lib")
 sys.path.append(RUNTIME_DIR)
 from PyRuntime import ExecutionSession
 
@@ -881,7 +880,7 @@ def prepare(cls, model, device='CPU', **kwargs):
         dynamic_inputs_dims = determine_dynamic_parameters(name)
         execute_commands([TEST_DRIVER, model_name], dynamic_inputs_dims)
         if not os.path.exists(exec_name) :
-            print("Failed " + test_config.TEST_DRIVER_COMMAND + ": " + name)
+            print("Failed " + test_config.TEST_DRIVER_PATH + ": " + name)
         return EndiannessAwareExecutionSession(exec_name,
                                                    "run_main_graph")
 

test/backend/test_config.py.in

@@ -1,4 +1,4 @@
-TEST_DRIVER_BUILD_PATH = "@TEST_DRIVER_BUILD_PATH@"
-TEST_DRIVER_COMMAND = "@TEST_DRIVER_COMMAND@"
-LLVM_PROJ_BUILD_PATH = "@LLVM_PROJ_BUILD@"
-CXX_PATH = "@CMAKE_CXX_COMPILER@"
+CXX_PATH = r"@CMAKE_CXX_COMPILER@"
+LLC_PATH = r"@LLVM_PROJ_BIN@/llc@CMAKE_EXECUTABLE_SUFFIX@"
+TEST_DRIVER_PATH = r"$<TARGET_FILE:onnx-mlir>"
+TEST_DRIVER_RUNTIME_PATH = r"@ONNX_MLIR_LIBRARY_PATH@"

test/mlir/CMakeLists.txt

@@ -1,7 +1,26 @@
 # SPDX-License-Identifier: Apache-2.0
 
+# Set LLVM_DEFAULT_EXTERNAL_LIT to an empty string to avoid warnings about the path
+# when using multi-config generators such as VS or Xcode
+set(LLVM_DEFAULT_EXTERNAL_LIT "")
 set(LLVM_LIT_OUTPUT_DIR ${LLVM_PROJ_BIN})
 
+# CMAKE_CFG_INTDIR is . for single-config generators such as make, and
+# it has a value (e.g. $(Configuration)) otherwise, so we can use it to
+# determine whether we are dealing with a multi-config generator.
+# For multi-config generators, this will replace $<CONFIG> with %(build_mode)s
+# which will in turn be replaced with the correct build mode at run time.
+# For single-config generators, this will replace $<CONFIG> with the correct
+# build mode at configuration time.
+# Both need to be done because configure_lit_site_cfg does not support generator
+# expressions.
+if (NOT "${CMAKE_CFG_INTDIR}" STREQUAL ".")
+  set_llvm_build_mode()
+  string(REPLACE "$<CONFIG>" "${LLVM_BUILD_MODE}" ONNX_MLIR_TOOLS_DIR "${ONNX_MLIR_RUNTIME_PATH}")
+else()
+  string(REPLACE "$<CONFIG>" "${CMAKE_BUILD_TYPE}" ONNX_MLIR_TOOLS_DIR "${ONNX_MLIR_RUNTIME_PATH}")
+endif()
+
 configure_lit_site_cfg(${CMAKE_CURRENT_SOURCE_DIR}/lit.site.cfg.py.in
                        ${CMAKE_CURRENT_BINARY_DIR}/lit.site.cfg.py
                        MAIN_CONFIG