[mlir] fix `add_tablegen()` macro to allow installing mlir-pdll

[ashay-github]

Summary

Prior to this patch, the add_tablegen() macro in
llvm/cmake/modules/TableGen.cmake added the install rule only if
project matched LLVM or MLIR. This patch adds an optional
DESTINATION argument, which, if non-empty, decides whether (and where)
to install the tablegen tool, thus eliminating the need for
project-specific overrides. This patch also updates all other
invocations of the add_tablegen() macro.

[nikic]

Funny that. I have a patch in testing that changes the same line to add an OR ${project} STREQUAL CLANG check, because clang-tblgen also needs to be exported. I think this indicates that this hardcoded list doesn't really make sense anymore, and we should instead allow the add_tablegen() call to specify whether this tablegen target should be exported or not. Something along the lines of add_tablegen(mlir-pdll MLIR_PDLL EXPORT) or add_tablegen(mlir-pdll MLIR_PDLL INSTALL).

Or, now that I look through the add_tablegen() calls we have, maybe we should just drop the project check entirely? This is still guarded by LLVM_BUILD_UTILS, so in any case people need to go out of their way to actually install tablegen binaries. (LLVM_BUILD_UTILS defaults to ON for the LLVM project, but will not get set when using a different build system entry point, e.g. a runtimes build.)

[ashay-github]

Thanks, that makes sense. How about something like add_tablegen(mlir-pdll INSTALL) (i.e. drop the project but add a boolean argument), so that we can still control the installation but not worry about the project name?

[ashay-github]

Removed the project check.

[ashay-github]

I'm not sure if the test failures (timeouts) are related to the changes in this patch.

Timed Out Tests (2):
  AddressSanitizer-x86_64-linux :: TestCases/scariness_score_test.cpp
  AddressSanitizer-x86_64-linux-dynamic :: TestCases/scariness_score_test.cpp

Testing Time: 640.70s
  Skipped          :    61
  Unsupported      :  2188
  Passed           : 93982
  Expectedly Failed:   281
  Timed Out        :     2

[nikic]

llvm/cmake/modules/TableGen.cmake

202

Hm, a problem is that this line is going to look for ${MLIR_PDLL_TOOLS_INSTALL_DIR}, so that variable would have to be set (presumably to ${MLIR_TOOLS_INSTALL_DIR}), otherwise this will end up in some kind of default location. The same problem would exist for (LLDB|LIBC|LLVM_LIBC)_TOOLS_INSTALL_DIR, all the other ones already define the variable.

Defining MLIR_PDLL_TOOLS_INSTALL_DIR is possible, but does seem somewhat ugly. A possibility would be to accept the install path as an argument (and skip installing if not provided):

add_tablegen(mlir-pdll MLIR_PDLL ... DESTINATION ${MLIR_TOOLS_INSTALL_DIR})

[ashay-github]

Added optional DESTINATION argument to add_tablegen(). Also updated all
invocations of the macro.

[ashay-github]

llvm/cmake/modules/TableGen.cmake

202

Thanks, that seems a lot more elegant than what I had before! Updated in the most recent version.

[nikic]

LGTM

llvm/cmake/modules/TableGen.cmake

147

I think it would be fine to write this as just cmake_parse_arguments(ADD_TABLEGEN "" "OPTIONS" "" ${ARGN}), but don't feel strongly about that.

[ashay-github]

Simply invocation of cmake_parse_arguments().

llvm/cmake/modules/TableGen.cmake

147

Thanks! Rewrote it as cmake_parse_arguments(ADD_TABLEGEN "" "DESTINATION" "" ${ARGN})

clang/utils/TableGen/CMakeLists.txt

 set(LLVM_LINK_COMPONENTS Support)
 
-add_tablegen(clang-tblgen CLANG
+add_tablegen(clang-tblgen CLANG DESTINATION "${CLANG_TOOLS_INSTALL_DIR}"
   ASTTableGen.cpp
   ClangASTNodesEmitter.cpp
   ClangASTPropertiesEmitter.cpp
   ClangAttrEmitter.cpp
   ClangCommentCommandInfoEmitter.cpp
   ClangCommentHTMLNamedCharacterReferenceEmitter.cpp
   ClangCommentHTMLTagsEmitter.cpp
   ClangDataCollectorsEmitter.cpp

llvm/cmake/modules/TableGen.cmake

function(add_public_tablegen_target target)
   if(LLVM_COMMON_DEPENDS)
     add_dependencies(${target} ${LLVM_COMMON_DEPENDS})
   endif()
   set_target_properties(${target} PROPERTIES FOLDER "Tablegenning")
   set(LLVM_COMMON_DEPENDS ${LLVM_COMMON_DEPENDS} ${target} PARENT_SCOPE)
 endfunction()
 
 macro(add_tablegen target project)
+  cmake_parse_arguments(ADD_TABLEGEN "" "DESTINATION" "" ${ARGN})
+
   set(${target}_OLD_LLVM_LINK_COMPONENTS ${LLVM_LINK_COMPONENTS})
   set(LLVM_LINK_COMPONENTS ${LLVM_LINK_COMPONENTS} TableGen)
 
   # CMake doesn't let compilation units depend on their dependent libraries on some generators.
   if(NOT CMAKE_GENERATOR MATCHES "Ninja" AND NOT XCODE)
     # FIXME: It leaks to user, callee of add_tablegen.
     set(LLVM_ENABLE_OBJLIB ON)
   endif()
 
-  add_llvm_executable(${target} DISABLE_LLVM_LINK_LLVM_DYLIB ${ARGN})
+  add_llvm_executable(${target} DISABLE_LLVM_LINK_LLVM_DYLIB
+    ${ADD_TABLEGEN_UNPARSED_ARGUMENTS})
   set(LLVM_LINK_COMPONENTS ${${target}_OLD_LLVM_LINK_COMPONENTS})
 
   set(${project}_TABLEGEN "${target}" CACHE
       STRING "Native TableGen executable. Saves building one when cross-compiling.")
 
   # Effective tblgen executable to be used:
   set(${project}_TABLEGEN_EXE ${${project}_TABLEGEN} PARENT_SCOPE)
   set(${project}_TABLEGEN_TARGET ${${project}_TABLEGEN} PARENT_SCOPE)
if( ${${project}_TABLEGEN} STREQUAL "${target}" )
       # If we're using the host tablegen, and utils were not requested, we have no
       # need to build this tablegen.
       if ( NOT LLVM_BUILD_UTILS )
         set_target_properties(${target} PROPERTIES EXCLUDE_FROM_ALL ON)
       endif()
     endif()
   endif()
 
-  if ((${project} STREQUAL LLVM OR ${project} STREQUAL MLIR) AND NOT LLVM_INSTALL_TOOLCHAIN_ONLY AND LLVM_BUILD_UTILS)
+  if (ADD_TABLEGEN_DESTINATION AND NOT LLVM_INSTALL_TOOLCHAIN_ONLY AND LLVM_BUILD_UTILS)
     set(export_to_llvmexports)
     if(${target} IN_LIST LLVM_DISTRIBUTION_COMPONENTS OR
         NOT LLVM_DISTRIBUTION_COMPONENTS)
       set(export_to_llvmexports EXPORT LLVMExports)
     endif()
 
     install(TARGETS ${target}
             ${export_to_llvmexports}
             COMPONENT ${target}
-            RUNTIME DESTINATION "${${project}_TOOLS_INSTALL_DIR}")
+            RUNTIME DESTINATION "${ADD_TABLEGEN_DESTINATION}")
     if(NOT LLVM_ENABLE_IDE)
       add_llvm_install_targets("install-${target}"
                                DEPENDS ${target}
                                COMPONENT ${target})
     endif()
   endif()
   set_property(GLOBAL APPEND PROPERTY LLVM_EXPORTS ${target})
 endmacro()

llvm/utils/TableGen/CMakeLists.txt

 add_subdirectory(GlobalISel)
 
 set(LLVM_LINK_COMPONENTS Support)
 
-add_tablegen(llvm-tblgen LLVM
+add_tablegen(llvm-tblgen LLVM DESTINATION "${LLVM_TOOLS_INSTALL_DIR}"
   AsmMatcherEmitter.cpp
   AsmWriterEmitter.cpp
   AsmWriterInst.cpp
   Attributes.cpp
   CallingConvEmitter.cpp
   CodeEmitterGen.cpp
   CodeGenDAGPatterns.cpp
   CodeGenHwModes.cpp

mlir/tools/mlir-pdll/CMakeLists.txt

 set(LLVM_LINK_COMPONENTS
   Demangle
   Support
   TableGen
 )
 
 set(LIBS
   MLIRIR
   MLIRPDLLAST
   MLIRPDLLCodeGen
   MLIRPDLLODS
   MLIRPDLLParser
   )
 
-add_tablegen(mlir-pdll MLIR_PDLL
+add_tablegen(mlir-pdll MLIR_PDLL DESTINATION "${MLIR_TOOLS_INSTALL_DIR}"
   mlir-pdll.cpp
 
   DEPENDS
   ${LIBS}
   )
 
 set_target_properties(mlir-pdll PROPERTIES FOLDER "Tablegenning")
 target_link_libraries(mlir-pdll PRIVATE ${LIBS})
 
 mlir_check_all_link_libraries(mlir-pdll)

mlir/tools/mlir-tblgen/CMakeLists.txt

 set(LLVM_LINK_COMPONENTS
   Demangle
   Support
   TableGen
 )
 
-add_tablegen(mlir-tblgen MLIR
+add_tablegen(mlir-tblgen MLIR DESTINATION "${MLIR_TOOLS_INSTALL_DIR}"
   AttrOrTypeDefGen.cpp
   AttrOrTypeFormatGen.cpp
   CodeGenHelpers.cpp
   DialectGen.cpp
   DirectiveCommonGen.cpp
   EnumsGen.cpp
   FormatGen.cpp
   LLVMIRConversionGen.cpp