[llvm] [TargetParser][cmake] Be Smarter about TableGen Deps (PR #144848)
Sam Elliott via llvm-commits
llvm-commits at lists.llvm.org
Thu Jun 19 06:58:09 PDT 2025
https://github.com/lenary updated https://github.com/llvm/llvm-project/pull/144848
>From 31da2c50755ec2090bbafbe517a19c816081508e Mon Sep 17 00:00:00 2001
From: Sam Elliott <quic_aelliott at quicinc.com>
Date: Wed, 18 Jun 2025 23:39:58 -0700
Subject: [PATCH 1/2] [TargetParser][cmake] Be Smarter about TableGen Deps
This tries to be a bit smarter for the OLD behaviour of CMP0116, to glob
more relevant directories looking for possible dependencies.
The changes are:
- Remove some duplication of lines in the `tablegen` function.
- Put CURRENT_SOURCE_DIR into `tblgen_includes` (at the front)
- Glob all directories in `tblgen_includes`
- Give up on `local_tds` which was wrong when using tablegen to compile
a file in a different directory (as TargetParser does)
- Use `EXTRA_INCLUDES` in TargetParser `tablegen` calls.
This is still an under-approximation of what might be included, at least
comparing the RISCVTargetParserDef.inc.d (after building
`target_parser_gen`), and the list of deps in the ninja file when
explicitly setting CMP0116 to OLD.
Fixes #144639
---
llvm/cmake/modules/TableGen.cmake | 58 ++++++++-----------
llvm/include/llvm/TargetParser/CMakeLists.txt | 6 +-
2 files changed, 27 insertions(+), 37 deletions(-)
diff --git a/llvm/cmake/modules/TableGen.cmake b/llvm/cmake/modules/TableGen.cmake
index b26fc62d4cc0c..da18b5ff23c83 100644
--- a/llvm/cmake/modules/TableGen.cmake
+++ b/llvm/cmake/modules/TableGen.cmake
@@ -21,6 +21,13 @@ function(tablegen project ofn)
message(FATAL_ERROR "${project}_TABLEGEN_EXE not set")
endif()
+ # Set the include directories
+ get_directory_property(tblgen_includes INCLUDE_DIRECTORIES)
+ list(PREPEND tblgen_includes ${ARG_EXTRA_INCLUDES})
+ list(PREPEND tblgen_includes ${CMAKE_CURRENT_SOURCE_DIR})
+ # Filter out any empty include items.
+ list(REMOVE_ITEM tblgen_includes "")
+
# Use depfile instead of globbing arbitrary *.td(s) for Ninja. We force
# CMake versions older than v3.30 on Windows to use the fallback behavior
# due to a depfile parsing bug on Windows paths in versions prior to 3.30.
@@ -42,22 +49,16 @@ function(tablegen project ofn)
-d ${ofn}.d
DEPFILE ${ofn}.d
)
- set(local_tds)
set(global_tds)
else()
- file(GLOB local_tds "*.td")
- file(GLOB_RECURSE global_tds "${LLVM_MAIN_INCLUDE_DIR}/llvm/*.td")
+ set(include_td_dirs "${tblgen_includes}")
+ list(TRANSFORM include_td_dirs APPEND "/*.td")
+ file(GLOB global_tds ${include_td_dirs})
set(additional_cmdline
-o ${CMAKE_CURRENT_BINARY_DIR}/${ofn}
)
endif()
- if (IS_ABSOLUTE ${LLVM_TARGET_DEFINITIONS})
- set(LLVM_TARGET_DEFINITIONS_ABSOLUTE ${LLVM_TARGET_DEFINITIONS})
- else()
- set(LLVM_TARGET_DEFINITIONS_ABSOLUTE
- ${CMAKE_CURRENT_SOURCE_DIR}/${LLVM_TARGET_DEFINITIONS})
- endif()
if (LLVM_ENABLE_DAGISEL_COV AND "-gen-dag-isel" IN_LIST ARGN)
list(APPEND LLVM_TABLEGEN_FLAGS "-instrument-coverage")
endif()
@@ -92,30 +93,11 @@ function(tablegen project ofn)
list(APPEND LLVM_TABLEGEN_FLAGS "-no-warn-on-unused-template-args")
endif()
- # We need both _TABLEGEN_TARGET and _TABLEGEN_EXE in the DEPENDS list
- # (both the target and the file) to have .inc files rebuilt on
- # a tablegen change, as cmake does not propagate file-level dependencies
- # of custom targets. See the following ticket for more information:
- # https://cmake.org/Bug/view.php?id=15858
- # The dependency on both, the target and the file, produces the same
- # dependency twice in the result file when
- # ("${${project}_TABLEGEN_TARGET}" STREQUAL "${${project}_TABLEGEN_EXE}")
- # but lets us having smaller and cleaner code here.
- get_directory_property(tblgen_includes INCLUDE_DIRECTORIES)
- list(APPEND tblgen_includes ${ARG_EXTRA_INCLUDES})
-
- # Get the current set of include paths for this td file.
- cmake_parse_arguments(ARG "" "" "DEPENDS;EXTRA_INCLUDES" ${ARGN})
- get_directory_property(tblgen_includes INCLUDE_DIRECTORIES)
- list(APPEND tblgen_includes ${ARG_EXTRA_INCLUDES})
- # Filter out any empty include items.
- list(REMOVE_ITEM tblgen_includes "")
-
- # Build the absolute path for the current input file.
if (IS_ABSOLUTE ${LLVM_TARGET_DEFINITIONS})
set(LLVM_TARGET_DEFINITIONS_ABSOLUTE ${LLVM_TARGET_DEFINITIONS})
else()
- set(LLVM_TARGET_DEFINITIONS_ABSOLUTE ${CMAKE_CURRENT_SOURCE_DIR}/${LLVM_TARGET_DEFINITIONS})
+ set(LLVM_TARGET_DEFINITIONS_ABSOLUTE
+ ${CMAKE_CURRENT_SOURCE_DIR}/${LLVM_TARGET_DEFINITIONS})
endif()
# Append this file and its includes to the compile commands file.
@@ -123,13 +105,21 @@ function(tablegen project ofn)
file(APPEND ${CMAKE_BINARY_DIR}/tablegen_compile_commands.yml
"--- !FileInfo:\n"
" filepath: \"${LLVM_TARGET_DEFINITIONS_ABSOLUTE}\"\n"
- " includes: \"${CMAKE_CURRENT_SOURCE_DIR};${tblgen_includes}\"\n"
+ " includes: \"${tblgen_includes}\"\n"
)
# Filter out empty items before prepending each entry with -I
- list(REMOVE_ITEM tblgen_includes "")
list(TRANSFORM tblgen_includes PREPEND -I)
+ # We need both _TABLEGEN_TARGET and _TABLEGEN_EXE in the DEPENDS list
+ # (both the target and the file) to have .inc files rebuilt on
+ # a tablegen change, as cmake does not propagate file-level dependencies
+ # of custom targets. See the following ticket for more information:
+ # https://cmake.org/Bug/view.php?id=15858
+ # The dependency on both, the target and the file, produces the same
+ # dependency twice in the result file when
+ # ("${${project}_TABLEGEN_TARGET}" STREQUAL "${${project}_TABLEGEN_EXE}")
+ # but lets us having smaller and cleaner code here.
set(tablegen_exe ${${project}_TABLEGEN_EXE})
set(tablegen_depends ${${project}_TABLEGEN_TARGET} ${tablegen_exe})
@@ -140,7 +130,7 @@ function(tablegen project ofn)
endif()
add_custom_command(OUTPUT ${CMAKE_CURRENT_BINARY_DIR}/${ofn}
- COMMAND ${tablegen_exe} ${ARG_UNPARSED_ARGUMENTS} -I ${CMAKE_CURRENT_SOURCE_DIR}
+ COMMAND ${tablegen_exe} ${ARG_UNPARSED_ARGUMENTS}
${tblgen_includes}
${LLVM_TABLEGEN_FLAGS}
${LLVM_TARGET_DEFINITIONS_ABSOLUTE}
@@ -150,7 +140,7 @@ function(tablegen project ofn)
# directory and local_tds may not contain it, so we must
# explicitly list it here:
DEPENDS ${ARG_DEPENDS} ${tablegen_depends}
- ${local_tds} ${global_tds}
+ ${global_tds}
${LLVM_TARGET_DEFINITIONS_ABSOLUTE}
${LLVM_TARGET_DEPENDS}
${LLVM_TABLEGEN_JOB_POOL}
diff --git a/llvm/include/llvm/TargetParser/CMakeLists.txt b/llvm/include/llvm/TargetParser/CMakeLists.txt
index b456da66a022f..4909acd09edf8 100644
--- a/llvm/include/llvm/TargetParser/CMakeLists.txt
+++ b/llvm/include/llvm/TargetParser/CMakeLists.txt
@@ -1,11 +1,11 @@
set(LLVM_TARGET_DEFINITIONS ${PROJECT_SOURCE_DIR}/lib/Target/ARM/ARM.td)
-tablegen(LLVM ARMTargetParserDef.inc -gen-arm-target-def -I ${PROJECT_SOURCE_DIR}/lib/Target/ARM/)
+tablegen(LLVM ARMTargetParserDef.inc -gen-arm-target-def EXTRA_INCLUDES ${PROJECT_SOURCE_DIR}/lib/Target/ARM)
set(LLVM_TARGET_DEFINITIONS ${PROJECT_SOURCE_DIR}/lib/Target/AArch64/AArch64.td)
-tablegen(LLVM AArch64TargetParserDef.inc -gen-arm-target-def -I ${PROJECT_SOURCE_DIR}/lib/Target/AArch64/)
+tablegen(LLVM AArch64TargetParserDef.inc -gen-arm-target-def EXTRA_INCLUDES ${PROJECT_SOURCE_DIR}/lib/Target/AArch64)
set(LLVM_TARGET_DEFINITIONS ${PROJECT_SOURCE_DIR}/lib/Target/RISCV/RISCV.td)
-tablegen(LLVM RISCVTargetParserDef.inc -gen-riscv-target-def -I ${PROJECT_SOURCE_DIR}/lib/Target/RISCV/)
+tablegen(LLVM RISCVTargetParserDef.inc -gen-riscv-target-def EXTRA_INCLUDES ${PROJECT_SOURCE_DIR}/lib/Target/RISCV)
# This covers all of the tablegen calls above.
add_public_tablegen_target(target_parser_gen)
>From eeaf686d7d57717cf1c319fb7c1efb2ab058c7cd Mon Sep 17 00:00:00 2001
From: Sam Elliott <quic_aelliott at quicinc.com>
Date: Thu, 19 Jun 2025 06:57:58 -0700
Subject: [PATCH 2/2] Re-add comment, fix comment
---
llvm/cmake/modules/TableGen.cmake | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/llvm/cmake/modules/TableGen.cmake b/llvm/cmake/modules/TableGen.cmake
index da18b5ff23c83..67a628d4953c3 100644
--- a/llvm/cmake/modules/TableGen.cmake
+++ b/llvm/cmake/modules/TableGen.cmake
@@ -93,6 +93,7 @@ function(tablegen project ofn)
list(APPEND LLVM_TABLEGEN_FLAGS "-no-warn-on-unused-template-args")
endif()
+ # Build the absolute path for the current input file.
if (IS_ABSOLUTE ${LLVM_TARGET_DEFINITIONS})
set(LLVM_TARGET_DEFINITIONS_ABSOLUTE ${LLVM_TARGET_DEFINITIONS})
else()
@@ -108,7 +109,7 @@ function(tablegen project ofn)
" includes: \"${tblgen_includes}\"\n"
)
- # Filter out empty items before prepending each entry with -I
+ # Prepend each include entry with -I for arguments.
list(TRANSFORM tblgen_includes PREPEND -I)
# We need both _TABLEGEN_TARGET and _TABLEGEN_EXE in the DEPENDS list
More information about the llvm-commits
mailing list