Skip to content

Commit 31da2c5

Browse files
committed
[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
1 parent a2ad656 commit 31da2c5

File tree

2 files changed

+27
-37
lines changed

2 files changed

+27
-37
lines changed

llvm/cmake/modules/TableGen.cmake

Lines changed: 24 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,13 @@ function(tablegen project ofn)
2121
message(FATAL_ERROR "${project}_TABLEGEN_EXE not set")
2222
endif()
2323

24+
# Set the include directories
25+
get_directory_property(tblgen_includes INCLUDE_DIRECTORIES)
26+
list(PREPEND tblgen_includes ${ARG_EXTRA_INCLUDES})
27+
list(PREPEND tblgen_includes ${CMAKE_CURRENT_SOURCE_DIR})
28+
# Filter out any empty include items.
29+
list(REMOVE_ITEM tblgen_includes "")
30+
2431
# Use depfile instead of globbing arbitrary *.td(s) for Ninja. We force
2532
# CMake versions older than v3.30 on Windows to use the fallback behavior
2633
# due to a depfile parsing bug on Windows paths in versions prior to 3.30.
@@ -42,22 +49,16 @@ function(tablegen project ofn)
4249
-d ${ofn}.d
4350
DEPFILE ${ofn}.d
4451
)
45-
set(local_tds)
4652
set(global_tds)
4753
else()
48-
file(GLOB local_tds "*.td")
49-
file(GLOB_RECURSE global_tds "${LLVM_MAIN_INCLUDE_DIR}/llvm/*.td")
54+
set(include_td_dirs "${tblgen_includes}")
55+
list(TRANSFORM include_td_dirs APPEND "/*.td")
56+
file(GLOB global_tds ${include_td_dirs})
5057
set(additional_cmdline
5158
-o ${CMAKE_CURRENT_BINARY_DIR}/${ofn}
5259
)
5360
endif()
5461

55-
if (IS_ABSOLUTE ${LLVM_TARGET_DEFINITIONS})
56-
set(LLVM_TARGET_DEFINITIONS_ABSOLUTE ${LLVM_TARGET_DEFINITIONS})
57-
else()
58-
set(LLVM_TARGET_DEFINITIONS_ABSOLUTE
59-
${CMAKE_CURRENT_SOURCE_DIR}/${LLVM_TARGET_DEFINITIONS})
60-
endif()
6162
if (LLVM_ENABLE_DAGISEL_COV AND "-gen-dag-isel" IN_LIST ARGN)
6263
list(APPEND LLVM_TABLEGEN_FLAGS "-instrument-coverage")
6364
endif()
@@ -92,44 +93,33 @@ function(tablegen project ofn)
9293
list(APPEND LLVM_TABLEGEN_FLAGS "-no-warn-on-unused-template-args")
9394
endif()
9495

95-
# We need both _TABLEGEN_TARGET and _TABLEGEN_EXE in the DEPENDS list
96-
# (both the target and the file) to have .inc files rebuilt on
97-
# a tablegen change, as cmake does not propagate file-level dependencies
98-
# of custom targets. See the following ticket for more information:
99-
# https://cmake.org/Bug/view.php?id=15858
100-
# The dependency on both, the target and the file, produces the same
101-
# dependency twice in the result file when
102-
# ("${${project}_TABLEGEN_TARGET}" STREQUAL "${${project}_TABLEGEN_EXE}")
103-
# but lets us having smaller and cleaner code here.
104-
get_directory_property(tblgen_includes INCLUDE_DIRECTORIES)
105-
list(APPEND tblgen_includes ${ARG_EXTRA_INCLUDES})
106-
107-
# Get the current set of include paths for this td file.
108-
cmake_parse_arguments(ARG "" "" "DEPENDS;EXTRA_INCLUDES" ${ARGN})
109-
get_directory_property(tblgen_includes INCLUDE_DIRECTORIES)
110-
list(APPEND tblgen_includes ${ARG_EXTRA_INCLUDES})
111-
# Filter out any empty include items.
112-
list(REMOVE_ITEM tblgen_includes "")
113-
114-
# Build the absolute path for the current input file.
11596
if (IS_ABSOLUTE ${LLVM_TARGET_DEFINITIONS})
11697
set(LLVM_TARGET_DEFINITIONS_ABSOLUTE ${LLVM_TARGET_DEFINITIONS})
11798
else()
118-
set(LLVM_TARGET_DEFINITIONS_ABSOLUTE ${CMAKE_CURRENT_SOURCE_DIR}/${LLVM_TARGET_DEFINITIONS})
99+
set(LLVM_TARGET_DEFINITIONS_ABSOLUTE
100+
${CMAKE_CURRENT_SOURCE_DIR}/${LLVM_TARGET_DEFINITIONS})
119101
endif()
120102

121103
# Append this file and its includes to the compile commands file.
122104
# This file is used by the TableGen LSP Language Server (tblgen-lsp-server).
123105
file(APPEND ${CMAKE_BINARY_DIR}/tablegen_compile_commands.yml
124106
"--- !FileInfo:\n"
125107
" filepath: \"${LLVM_TARGET_DEFINITIONS_ABSOLUTE}\"\n"
126-
" includes: \"${CMAKE_CURRENT_SOURCE_DIR};${tblgen_includes}\"\n"
108+
" includes: \"${tblgen_includes}\"\n"
127109
)
128110

129111
# Filter out empty items before prepending each entry with -I
130-
list(REMOVE_ITEM tblgen_includes "")
131112
list(TRANSFORM tblgen_includes PREPEND -I)
132113

114+
# We need both _TABLEGEN_TARGET and _TABLEGEN_EXE in the DEPENDS list
115+
# (both the target and the file) to have .inc files rebuilt on
116+
# a tablegen change, as cmake does not propagate file-level dependencies
117+
# of custom targets. See the following ticket for more information:
118+
# https://cmake.org/Bug/view.php?id=15858
119+
# The dependency on both, the target and the file, produces the same
120+
# dependency twice in the result file when
121+
# ("${${project}_TABLEGEN_TARGET}" STREQUAL "${${project}_TABLEGEN_EXE}")
122+
# but lets us having smaller and cleaner code here.
133123
set(tablegen_exe ${${project}_TABLEGEN_EXE})
134124
set(tablegen_depends ${${project}_TABLEGEN_TARGET} ${tablegen_exe})
135125

@@ -140,7 +130,7 @@ function(tablegen project ofn)
140130
endif()
141131

142132
add_custom_command(OUTPUT ${CMAKE_CURRENT_BINARY_DIR}/${ofn}
143-
COMMAND ${tablegen_exe} ${ARG_UNPARSED_ARGUMENTS} -I ${CMAKE_CURRENT_SOURCE_DIR}
133+
COMMAND ${tablegen_exe} ${ARG_UNPARSED_ARGUMENTS}
144134
${tblgen_includes}
145135
${LLVM_TABLEGEN_FLAGS}
146136
${LLVM_TARGET_DEFINITIONS_ABSOLUTE}
@@ -150,7 +140,7 @@ function(tablegen project ofn)
150140
# directory and local_tds may not contain it, so we must
151141
# explicitly list it here:
152142
DEPENDS ${ARG_DEPENDS} ${tablegen_depends}
153-
${local_tds} ${global_tds}
143+
${global_tds}
154144
${LLVM_TARGET_DEFINITIONS_ABSOLUTE}
155145
${LLVM_TARGET_DEPENDS}
156146
${LLVM_TABLEGEN_JOB_POOL}
Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,11 @@
11
set(LLVM_TARGET_DEFINITIONS ${PROJECT_SOURCE_DIR}/lib/Target/ARM/ARM.td)
2-
tablegen(LLVM ARMTargetParserDef.inc -gen-arm-target-def -I ${PROJECT_SOURCE_DIR}/lib/Target/ARM/)
2+
tablegen(LLVM ARMTargetParserDef.inc -gen-arm-target-def EXTRA_INCLUDES ${PROJECT_SOURCE_DIR}/lib/Target/ARM)
33

44
set(LLVM_TARGET_DEFINITIONS ${PROJECT_SOURCE_DIR}/lib/Target/AArch64/AArch64.td)
5-
tablegen(LLVM AArch64TargetParserDef.inc -gen-arm-target-def -I ${PROJECT_SOURCE_DIR}/lib/Target/AArch64/)
5+
tablegen(LLVM AArch64TargetParserDef.inc -gen-arm-target-def EXTRA_INCLUDES ${PROJECT_SOURCE_DIR}/lib/Target/AArch64)
66

77
set(LLVM_TARGET_DEFINITIONS ${PROJECT_SOURCE_DIR}/lib/Target/RISCV/RISCV.td)
8-
tablegen(LLVM RISCVTargetParserDef.inc -gen-riscv-target-def -I ${PROJECT_SOURCE_DIR}/lib/Target/RISCV/)
8+
tablegen(LLVM RISCVTargetParserDef.inc -gen-riscv-target-def EXTRA_INCLUDES ${PROJECT_SOURCE_DIR}/lib/Target/RISCV)
99

1010
# This covers all of the tablegen calls above.
1111
add_public_tablegen_target(target_parser_gen)

0 commit comments

Comments
 (0)