[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:25 PST 2023
https://github.com/akirchhoff-modular created https://github.com/llvm/llvm-project/pull/72333
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.
>From e5eac84c23cd876c7977b30d7d30ccf52ae385d8 Mon Sep 17 00:00:00 2001
From: Alex Kirchhoff <akirchhoff at modular.com>
Date: Tue, 14 Nov 2023 16:44:27 -0800
Subject: [PATCH] [CMake][TableGen] Make TableGen CMake functions compatible
with CMP0116
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.
---
llvm/cmake/modules/TableGen.cmake | 39 +++++++++++++++++++++----------
mlir/cmake/modules/AddMLIR.cmake | 39 +++++++++++++++++++++----------
2 files changed, 54 insertions(+), 24 deletions(-)
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()
More information about the Mlir-commits
mailing list