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

[Ashay Rane via Phabricator via cfe-commits]

ashay-github marked an inline comment as done.
ashay-github added inline comments.


================
Comment at: llvm/cmake/modules/TableGen.cmake:199
             COMPONENT ${target}
             RUNTIME DESTINATION "${${project}_TOOLS_INSTALL_DIR}")
     if(NOT LLVM_ENABLE_IDE)
----------------
nikic wrote:
> 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})
> ```
Thanks, that seems a lot more elegant than what I had before! Updated in the most recent version.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D131282/new/

https://reviews.llvm.org/D131282