[Mlir-commits] [llvm] [mlir] [CMake][TableGen] Make TableGen CMake functions compatible with CMP0116 (PR #72333)
llvmlistbot at llvm.org
llvmlistbot at llvm.org
Tue Nov 14 17:04:55 PST 2023
llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT-->
@llvm/pr-subscribers-mlir
Author: None (akirchhoff-modular)
<details>
<summary>Changes</summary>
By default, `add_custom_command` rules execute within their defining directory's corresponding build directory. When `DEPFILE` is used with Ninja, this causes a discrepancy: Ninja expects the path in the depfile to be a relative path from the root of the build directory, but since the working directory is changed by default during the execution of the rule, the path won't match up, and Ninja will conservatively ignore the depfile.
Historically, this is the only way it worked, and CMakeLists.txt files would have to be written to work around this, manually converting paths to build-root-directory-relative and changing the working directory of the command to match.
Starting with CMake 3.20, CMake introduced a native workaround, controlled by policy CMP0116. When CMP0116 is set to NEW, CMake will automatically translate depfiles such that CMakeLists.txt no longer need to be written taking this into account. This behavior is incompatible with the manual workaround. LLVM sets CMP0116 to OLD in `cmake/Modules/CMakePolicy.cmake`, so when building LLVM, this is not a problem -- we turn off CMake's native workaround and work around it ourselves.
Since the TableGen CMake modules are also installed along with LLVM, downstream projects that use LLVM and its TableGen CMake rules are also forced to adopt CMP0116 to be set to OLD. This causes conflicts when other dependencies of such downstream projects require CMP0116 to be set to NEW. CMake policy scopes can help with this, but it is tedious to handle, as a policy scope would need to be introduced whenever the TableGen rule is used, and it requires the caller to understand an implementation detail of the TableGen rule.
Instead, this PR changes the TableGen CMake rules such that they will inspect the current state of CMP0116, and only conditionally apply the manual workaround. This way, the TableGen rules will work regardless of the setting of CMP0116.
In the future, potentially the value of CMP0116 set in `CMakePolicy.cmake` could change to bring LLVM up-to-date with CMake default behavior. However, I am leaving out such a default change to reduce any possibility of breakage for other downstream projects at this time.
---
Full diff: https://github.com/llvm/llvm-project/pull/72333.diff
2 Files Affected:
- (modified) llvm/cmake/modules/TableGen.cmake (+27-12)
- (modified) mlir/cmake/modules/AddMLIR.cmake (+27-12)
``````````diff
diff --git a/llvm/cmake/modules/TableGen.cmake b/llvm/cmake/modules/TableGen.cmake
index 7fd6628ef55d33a..0f6aa0c5d826ab3 100644
--- a/llvm/cmake/modules/TableGen.cmake
+++ b/llvm/cmake/modules/TableGen.cmake
@@ -19,19 +19,34 @@ function(tablegen project ofn)
# Use depfile instead of globbing arbitrary *.td(s) for Ninja.
if(CMAKE_GENERATOR MATCHES "Ninja")
- # Make output path relative to build.ninja, assuming located on
- # ${CMAKE_BINARY_DIR}.
# CMake emits build targets as relative paths but Ninja doesn't identify
- # absolute path (in *.d) as relative path (in build.ninja)
- # Note that tblgen is executed on ${CMAKE_BINARY_DIR} as working directory.
- file(RELATIVE_PATH ofn_rel
- ${CMAKE_BINARY_DIR} ${CMAKE_CURRENT_BINARY_DIR}/${ofn})
- set(additional_cmdline
- -o ${ofn_rel}
- -d ${ofn_rel}.d
- WORKING_DIRECTORY ${CMAKE_BINARY_DIR}
- DEPFILE ${CMAKE_CURRENT_BINARY_DIR}/${ofn}.d
- )
+ # absolute path (in *.d) as relative path (in build.ninja). If CMP0116 is
+ # NEW, CMake handles this discrepancy for us -- otherwise, we have to work
+ # around it ourselves.
+ if(POLICY CMP0116)
+ cmake_policy(GET CMP0116 cmp0116_state)
+ else()
+ set(cmp0116_state OLD)
+ endif()
+ if(cmp0116_state STREQUAL NEW)
+ set(additional_cmdline
+ -o ${ofn}
+ -d ${ofn}.d
+ DEPFILE ${ofn}.d
+ )
+ else()
+ # Make output path relative to build.ninja, assuming located on
+ # ${CMAKE_BINARY_DIR}.
+ # Note that tblgen is executed on ${CMAKE_BINARY_DIR} as working directory.
+ file(RELATIVE_PATH ofn_rel
+ ${CMAKE_BINARY_DIR} ${CMAKE_CURRENT_BINARY_DIR}/${ofn})
+ set(additional_cmdline
+ -o ${ofn_rel}
+ -d ${ofn_rel}.d
+ WORKING_DIRECTORY ${CMAKE_BINARY_DIR}
+ DEPFILE ${CMAKE_CURRENT_BINARY_DIR}/${ofn}.d
+ )
+ endif()
set(local_tds)
set(global_tds)
else()
diff --git a/mlir/cmake/modules/AddMLIR.cmake b/mlir/cmake/modules/AddMLIR.cmake
index 544abe43688820e..714054f17515ca7 100644
--- a/mlir/cmake/modules/AddMLIR.cmake
+++ b/mlir/cmake/modules/AddMLIR.cmake
@@ -48,19 +48,34 @@ function(_pdll_tablegen project ofn)
# Use depfile instead of globbing arbitrary *.td(s) for Ninja.
if(CMAKE_GENERATOR MATCHES "Ninja")
- # Make output path relative to build.ninja, assuming located on
- # ${CMAKE_BINARY_DIR}.
# CMake emits build targets as relative paths but Ninja doesn't identify
- # absolute path (in *.d) as relative path (in build.ninja)
- # Note that tblgen is executed on ${CMAKE_BINARY_DIR} as working directory.
- file(RELATIVE_PATH ofn_rel
- ${CMAKE_BINARY_DIR} ${CMAKE_CURRENT_BINARY_DIR}/${ofn})
- set(additional_cmdline
- -o ${ofn_rel}
- -d ${ofn_rel}.d
- WORKING_DIRECTORY ${CMAKE_BINARY_DIR}
- DEPFILE ${CMAKE_CURRENT_BINARY_DIR}/${ofn}.d
- )
+ # absolute path (in *.d) as relative path (in build.ninja). If CMP0116 is
+ # NEW, CMake handles this discrepancy for us -- otherwise, we have to work
+ # around it ourselves.
+ if(POLICY CMP0116)
+ cmake_policy(GET CMP0116 cmp0116_state)
+ else()
+ set(cmp0116_state OLD)
+ endif()
+ if(cmp0116_state STREQUAL NEW)
+ set(additional_cmdline
+ -o ${ofn}
+ -d ${ofn}.d
+ DEPFILE ${ofn}.d
+ )
+ else()
+ # Make output path relative to build.ninja, assuming located on
+ # ${CMAKE_BINARY_DIR}.
+ # Note that tblgen is executed on ${CMAKE_BINARY_DIR} as working directory.
+ file(RELATIVE_PATH ofn_rel
+ ${CMAKE_BINARY_DIR} ${CMAKE_CURRENT_BINARY_DIR}/${ofn})
+ set(additional_cmdline
+ -o ${ofn_rel}
+ -d ${ofn_rel}.d
+ WORKING_DIRECTORY ${CMAKE_BINARY_DIR}
+ DEPFILE ${CMAKE_CURRENT_BINARY_DIR}/${ofn}.d
+ )
+ endif()
set(local_tds)
set(global_tds)
else()
``````````
</details>
https://github.com/llvm/llvm-project/pull/72333
More information about the Mlir-commits
mailing list