[mlir] Support cmake 3.15+ in the python mlir configuration

[stella.stamenova]

Summary

https://reviews.llvm.org/D128230 updated the mlir python configuration to depend heavily on interface libraries and their properties. Unfortunately, some of these are not available in cmake 3.15 (which is the lowest supported version for mlir). This change modifies the setup of the interface libraries to work in cmake 3.15.

More specifically, the issue is that both INCLUDE_DIRECTORIES and SOURCES are only supported on interface libraries in 3.15 in their "interface" versions, so we cannot set plain INCLUDE_DIRECTORIES and SOURCES. We were setting these to work around the fact that we "build" the mlir sources targets at configuration time (instead of build time) and the generator expressions are not yet evaluated at that point. This change removes these "plain" variants of the properties and instead adds custom evaluation for the generator expressions. This is not really great (neither was the other workaround, really), so we should plan to upgrade to FILE_SETS or similar as soon as possible (which depends on requiring a much newer version of cmake). Other options include always copying the python files instead of creating symbolic links.

This change also fixes a dependency issue between the python files generated through tablegen and the targets that copy the files. The dependency was previously implied via the SOURCES property, however, there really should be an explicit target dependency per the cmake documentation (see the OUTPUT property of https://cmake.org/cmake/help/latest/command/add_custom_command.html).

[stella.stamenova]

mlir/cmake/modules/AddMLIRPython.cmake

333

I think this function should be removed entirely and instead declare_mlir_dialect_python_bindings should be modified to support the extension naming scheme because they are almost identical, but I didn't want to do it in this change.

[stellaraccident]

Oof... thank you for looking into this, but this is also pretty gross/fragile. Are we *sure* we want to do this? This is the only project I work on that treats CMake as an LTS-like thing, and I would be utterly shocked if this was the only optional feature that found itself needing a newer CMake for basic things. Do you happen to know which CMake version this workaround is not needed for (I knew at one time but have forgotten)?

[stella.stamenova]

In D129163#3631374, @stellaraccident wrote:

Oof... thank you for looking into this, but this is also pretty gross/fragile. Are we *sure* we want to do this? This is the only project I work on that treats CMake as an LTS-like thing, and I would be utterly shocked if this was the only optional feature that found itself needing a newer CMake for basic things. Do you happen to know which CMake version this workaround is not needed for (I knew at one time but have forgotten)?

I think it would be much better to upgrade the CMake version than try to support older versions not just for this but in general! I also did want to make sure we could support 3.15 if we wanted. The interface library works great in 3.15 for the code that we compile (that path has no changes to work in 3.15), but the path with the copy/symlinks is worse as you can see.

I did some testing and 3.18.3 does not work with the current setup, but 3.19 does. I'm not sure if there's a version in between that does.

[stella.stamenova]

This is obsolete since https://reviews.llvm.org/D130171 was committed

mlir/cmake/modules/AddMLIRPython.cmake

 #     For relocatable sources, this will point deeper into the directory that
 #     can be relocated. For generated sources, can be relative to
 #     CMAKE_CURRENT_BINARY_DIR. Generated and non generated sources cannot be
 #     mixed.
 #   ADD_TO_PARENT: Adds this source grouping to a previously declared source
 #     grouping. Source groupings form a DAG.
 #   SOURCES: List of specific source files relative to ROOT_DIR to include.
 #   SOURCES_GLOB: List of glob patterns relative to ROOT_DIR to include.
+#   DEPENDS: Additional dependency targets.
 function(declare_mlir_python_sources name)
   cmake_parse_arguments(ARG
     ""
     "ROOT_DIR;ADD_TO_PARENT"
-    "SOURCES;SOURCES_GLOB"
+    "SOURCES;SOURCES_GLOB;DEPENDS"
     ${ARGN})
 
   if(NOT ARG_ROOT_DIR)
     set(ARG_ROOT_DIR "${CMAKE_CURRENT_SOURCE_DIR}")
   endif()
   set(_install_destination "src/python/${name}")
 
   # Process the glob.
   set(_glob_sources)
   if(ARG_SOURCES_GLOB)
     set(_glob_spec ${ARG_SOURCES_GLOB})
     list(TRANSFORM _glob_spec PREPEND "${ARG_ROOT_DIR}/")
     file(GLOB_RECURSE _glob_sources
       RELATIVE "${ARG_ROOT_DIR}"
       ${_glob_spec}
     )
     list(APPEND ARG_SOURCES ${_glob_sources})
   endif()
 
   # We create a custom target to carry properties and dependencies for
   # generated sources.
   add_library(${name} INTERFACE)
+  if(ARG_DEPENDS)
+    add_dependencies(${name} ${ARG_DEPENDS})
+  endif()
+
   set_target_properties(${name} PROPERTIES
     # Yes: Leading-lowercase property names are load bearing and the recommended
     # way to do this: https://gitlab.kitware.com/cmake/cmake/-/issues/19261
     EXPORT_PROPERTIES "mlir_python_SOURCES_TYPE;mlir_python_DEPENDS"
     mlir_python_SOURCES_TYPE pure
     mlir_python_DEPENDS ""
   )
 
   # Use the interface include directories and sources on the target to carry the
   # properties we would like to export. These support generator expressions and
   # allow us to properly specify paths in both the local build and install scenarios.
   # The one caveat here is that because we don't directly build against the interface
-  # library, we need to specify the INCLUDE_DIRECTORIES and SOURCES properties as well
-  # via private properties because the evaluation would happen at configuration time
-  # instead of build time.
-  # Eventually this could be done using a FILE_SET simplifying the logic below.
+  # library, we will need to manually evaluate the expressions at configuration time.
+  # Eventually this could be done using a FILE_SET to simplify the evaluation logic.
   # FILE_SET is available in cmake 3.23+, so it is not an option at the moment.
   target_include_directories(${name} INTERFACE
     "$<BUILD_INTERFACE:${ARG_ROOT_DIR}>"
     "$<INSTALL_INTERFACE:${_install_destination}>"
   )
-  set_property(TARGET ${name} PROPERTY INCLUDE_DIRECTORIES ${ARG_ROOT_DIR})
 
   if(ARG_SOURCES)
-    list(TRANSFORM ARG_SOURCES PREPEND "${ARG_ROOT_DIR}/" OUTPUT_VARIABLE _build_sources)
-    list(TRANSFORM ARG_SOURCES PREPEND "${_install_destination}/" OUTPUT_VARIABLE _install_sources)
+    foreach(_src ${ARG_SOURCES})
       target_sources(${name}
         INTERFACE
-        "$<INSTALL_INTERFACE:${_install_sources}>"
-        "$<BUILD_INTERFACE:${_build_sources}>"
-      PRIVATE ${_build_sources}
+          "$<INSTALL_INTERFACE:${_install_destination}/${_src}>"
+          "$<BUILD_INTERFACE:${ARG_ROOT_DIR}/${_src}>"
       )
+    endforeach()
   endif()
 
   # Add to parent.
   if(ARG_ADD_TO_PARENT)
     set_property(TARGET ${ARG_ADD_TO_PARENT} APPEND PROPERTY mlir_python_DEPENDS ${name})
   endif()
 
   # Install.
cmake_parse_arguments(ARG
     ${ARGN})
   # Sources.
   set(_dialect_target "${ARG_ADD_TO_PARENT}.${ARG_DIALECT_NAME}")
   declare_mlir_python_sources(${_dialect_target}
     ROOT_DIR "${ARG_ROOT_DIR}"
     ADD_TO_PARENT "${ARG_ADD_TO_PARENT}"
     SOURCES "${ARG_SOURCES}"
     SOURCES_GLOB "${ARG_SOURCES_GLOB}"
+    DEPENDS ${ARG_DEPENDS}
   )
 
   # Tablegen
   if(ARG_TD_FILE)
     set(tblgen_target "${_dialect_target}.tablegen")
     set(td_file "${ARG_ROOT_DIR}/${ARG_TD_FILE}")
     get_filename_component(relative_td_directory "${ARG_TD_FILE}" DIRECTORY)
     file(MAKE_DIRECTORY "${CMAKE_CURRENT_BINARY_DIR}/${relative_td_directory}")
     set(dialect_filename "${relative_td_directory}/_${ARG_DIALECT_NAME}_ops_gen.py")
     set(LLVM_TARGET_DEFINITIONS ${td_file})
     mlir_tablegen("${dialect_filename}"
       -gen-python-op-bindings -bind-dialect=${ARG_DIALECT_NAME}
-      DEPENDS ${ARG_DEPENDS}
     )
     add_public_tablegen_target(${tblgen_target})
+    if(ARG_DEPENDS)
+      add_dependencies(${tblgen_target} ${ARG_DEPENDS})
+    endif()
 
     # Generated.
     declare_mlir_python_sources("${_dialect_target}.ops_gen"
       ROOT_DIR "${CMAKE_CURRENT_BINARY_DIR}"
       ADD_TO_PARENT "${_dialect_target}"
       SOURCES "${dialect_filename}"
+      DEPENDS ${tblgen_target}
     )
   endif()
 endfunction()
 
 # Function: declare_mlir_dialect_extension_python_bindings
 # Helper to generate source groups for dialect extensions, including both
 # static source files and a TD_FILE to generate wrappers.
 #
 # This will generate a source group named ${ADD_TO_PARENT}.${EXTENSION_NAME}.
 #
 # Arguments:
 #   ROOT_DIR: Same as for declare_mlir_python_sources().
 #   ADD_TO_PARENT: Same as for declare_mlir_python_sources(). Unique names
 #     for the subordinate source groups are derived from this.
 #   TD_FILE: Tablegen file to generate source for (relative to ROOT_DIR).
 #   DIALECT_NAME: Python name of the dialect.
 #   EXTENSION_NAME: Python name of the dialect extension.
 #   SOURCES: Same as declare_mlir_python_sources().
 #   SOURCES_GLOB: Same as declare_mlir_python_sources().
 #   DEPENDS: Additional dependency targets.
 function(declare_mlir_dialect_extension_python_bindings)
   cmake_parse_arguments(ARG
     ""
     "ROOT_DIR;ADD_TO_PARENT;TD_FILE;DIALECT_NAME;EXTENSION_NAME"
     "SOURCES;SOURCES_GLOB;DEPENDS"
     ${ARGN})
   # Source files.
   set(_extension_target "${ARG_ADD_TO_PARENT}.${ARG_EXTENSION_NAME}")
   declare_mlir_python_sources(${_extension_target}
     ROOT_DIR "${ARG_ROOT_DIR}"
     ADD_TO_PARENT "${ARG_ADD_TO_PARENT}"
     SOURCES "${ARG_SOURCES}"
     SOURCES_GLOB "${ARG_SOURCES_GLOB}"
+    DEPENDS ${ARG_DEPENDS}
   )
 
   # Tablegen
   if(ARG_TD_FILE)
     set(tblgen_target "${ARG_ADD_TO_PARENT}.${ARG_EXTENSION_NAME}.tablegen")
     set(td_file "${ARG_ROOT_DIR}/${ARG_TD_FILE}")
     get_filename_component(relative_td_directory "${ARG_TD_FILE}" DIRECTORY)
     file(MAKE_DIRECTORY "${CMAKE_CURRENT_BINARY_DIR}/${relative_td_directory}")
     set(output_filename "${relative_td_directory}/_${ARG_EXTENSION_NAME}_ops_gen.py")
     set(LLVM_TARGET_DEFINITIONS ${td_file})
-    mlir_tablegen("${output_filename}" -gen-python-op-bindings
-                  -bind-dialect=${ARG_DIALECT_NAME}
-                  -dialect-extension=${ARG_EXTENSION_NAME})
+    mlir_tablegen("${output_filename}"
+      -gen-python-op-bindings -bind-dialect=${ARG_DIALECT_NAME}
+      -dialect-extension=${ARG_EXTENSION_NAME}
+    )
     add_public_tablegen_target(${tblgen_target})
     if(ARG_DEPENDS)
       add_dependencies(${tblgen_target} ${ARG_DEPENDS})
     endif()
 
     declare_mlir_python_sources("${_extension_target}.ops_gen"
       ROOT_DIR "${CMAKE_CURRENT_BINARY_DIR}"
       ADD_TO_PARENT "${_extension_target}"
       SOURCES "${output_filename}"
+      DEPENDS ${tblgen_target}
     )
   endif()
 endfunction()
 
 # Function: mlir_python_setup_extension_rpath
 # Sets RPATH properties on a target, assuming that it is being output to
 # an _mlir_libs directory with all other libraries. For static linkage,
 # the RPATH will just be the origin. If linking dynamically, then the LLVM
if(CMAKE_HOST_WIN32)
     set(_link_or_copy copy_if_different)
   else()
     set(_link_or_copy create_symlink)
   endif()
 
   foreach(_sources_target ${ARG_SOURCES_TARGETS})
     add_dependencies(${name} ${_sources_target})
 
-    get_target_property(_src_paths ${_sources_target} SOURCES)
-    if(NOT _src_paths)
-      get_target_property(_src_paths ${_sources_target} INTERFACE_SOURCES)
-      if(NOT _src_paths)
-        break()
-      endif()
-    endif()
+    # We use interface libraries to carry information about the sources via the
+    # INTERFACE_INCLUDE_DIRECTORIES and INTERFACE_SOURCES properties. These support
+    # generator expressions and allows us to specify the paths for both the local and install
+    # scenarios. However, because the setup below happens at *configuration* time instead of
+    # build time, the generator expressions have not yet been evaluated in the local build
+    # case. To work around this, we do our own "evaluation" of the BUILD_INTERFACE expression.
+    # Eventually this could be done using a FILE_SET simplifying the logic below.
+    # FILE_SET is available in cmake 3.23+, so it is not an option at the moment.
+    set(_build_regex "^\\$\<BUILD\_INTERFACE\:(.*)\>$")
+    set(_install_regex "^\\$\<INSTALL\_INTERFACE\:(.*)\>$")
+
+    set(_src_paths)
+    get_target_property(_interface_src_paths ${_sources_target} INTERFACE_SOURCES)
+    if(_interface_src_paths)
+      foreach(_interface_src_path ${_interface_src_paths})
+        if(NOT _interface_src_path MATCHES ${_build_regex} AND NOT _interface_src_path MATCHES ${_install_regex})
+          list(APPEND _src_paths ${_interface_src_path})
+        elseif(_interface_src_path MATCHES ${_build_regex})
+          string(REGEX REPLACE ${_build_regex} "\\1" _build_src_path ${_interface_src_path})
+          list(APPEND _src_paths ${_build_src_path})
+        endif()
+      endforeach()
+    endif()
 
-    get_target_property(_root_dir ${_sources_target} INCLUDE_DIRECTORIES)
-    if(NOT _root_dir)
-      get_target_property(_root_dir ${_sources_target} INTERFACE_INCLUDE_DIRECTORIES)
+    set(_root_dir)
+    get_target_property(_interface_root_dirs ${_sources_target} INTERFACE_INCLUDE_DIRECTORIES)
+    if(_interface_root_dirs)
+      foreach(_interface_root_dir ${_interface_root_dirs})
+        if(NOT _interface_root_dir MATCHES ${_build_regex} AND NOT _interface_root_dir MATCHES ${_install_regex})
+          set(_root_dir ${_interface_root_dir})
+        elseif(_interface_root_dir MATCHES ${_build_regex})
+          string(REGEX REPLACE ${_build_regex} "\\1" _root_dir ${_interface_root_dir})
+        endif()
+      endforeach()
     endif()
 
     foreach(_src_path ${_src_paths})
       file(RELATIVE_PATH _source_relative_path "${_root_dir}" "${_src_path}")
       set(_dest_path "${ARG_OUTPUT_DIRECTORY}/${_source_relative_path}")
 
       get_filename_component(_dest_dir "${_dest_path}" DIRECTORY)
       file(MAKE_DIRECTORY "${_dest_dir}")

mlir/python/CMakeLists.txt

mlir_tablegen(
     "dialects/_python_test_ops_gen.py"
     -gen-python-op-bindings
     -bind-dialect=python_test)
   add_public_tablegen_target(PythonTestDialectPyIncGen)
   declare_mlir_python_sources(
     MLIRPythonTestSources.Dialects.PythonTest.ops_gen
     ROOT_DIR "${CMAKE_CURRENT_BINARY_DIR}"
     ADD_TO_PARENT MLIRPythonTestSources.Dialects.PythonTest
-    SOURCES "dialects/_python_test_ops_gen.py")
+    SOURCES "dialects/_python_test_ops_gen.py"
+    DEPENDS PythonTestDialectPyIncGen)
 
   declare_mlir_python_extension(MLIRPythonTestSources.PythonTestExtension
     MODULE_NAME _mlirPythonTest
     ADD_TO_PARENT MLIRPythonTestSources.Dialects
     ROOT_DIR "${MLIR_SOURCE_DIR}/test/python/lib"
     SOURCES
       PythonTestModule.cpp
     PRIVATE_LINK_LIBS