[TargetParser][cmake] Be Smarter about TableGen Deps (#144848)

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

Author: lenary

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,44 +93,34 @@ 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.
   # This file is used by the TableGen LSP Language Server (tblgen-lsp-server).
   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 "")
+  # 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
+  # (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 +131,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 +141,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}

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)
 
 set(LLVM_TARGET_DEFINITIONS ${PROJECT_SOURCE_DIR}/lib/Target/PowerPC/PPC.td)
 tablegen(LLVM PPCGenTargetFeatures.inc -gen-target-features -I${PROJECT_SOURCE_DIR}/lib/Target/PowerPC)