| Issue |
125779
|
| Summary |
Can recommendation for out-of-tree projects to `add_definitions(${LLVM_DEFINITIONS})` be retired?
|
| Labels |
|
| Assignees |
|
| Reporter |
christopherbate
|
TLDR: Recently I spent several hours debugging compilation errors caused by differences in how a bunch of LLVM and MLIR-related projects, both in the OSS world and local to my employer, utilize the `LLVM_DEFINITIONS` CMake variable when finding and setting LLVM or MLIR in an out-of-tree CMake project. Rather than fixing them all independently to be uniform, I'm wondering why downstream projects need to utilize `LLVM_DEFINITIONS` at all (see below for the LLVM CMake doc guidance on this variable).
The `LLVM_DEFINITIONS` variable is defined like this in `HandleLLVMOptions.cmake`:
```
function(get_compile_definitions)
get_directory_property(top_dir_definitions DIRECTORY ${CMAKE_SOURCE_DIR} COMPILE_DEFINITIONS)
foreach(definition ${top_dir_definitions})
if(DEFINED result)
string(APPEND result " -D${definition}")
else()
set(result "-D${definition}")
endif()
endforeach()
set(LLVM_DEFINITIONS "${result}" PARENT_SCOPE)
endfunction()
get_compile_definitions()
```
So apparently, it just gets all the directory COMPILE_DEFINITIONS in the top-level source directory and joins them into a space-separated list as a string, adding the `-D` prefix.
Searching uses of `LLVM_DEFINITIONS` in both in-tree and out-of-tree projects like StableHLO [see use here](https://github.com/openxla/stablehlo/blob/main/CMakeLists.txt#L176) , shows that many callers are invoking `add_definitions(${LLVM_DEFINITIONS})`, which is wrong because `add_definitions("-Dname1=1 -Dname2=2")` will result in `-Dname1='1 -Dname2=2'` being added to the compiler flags.
Because of this, the official [LLVM CMake documentation section "Embedding LLVM in your project"](https://llvm.org/docs/CMake.html#embedding-llvm-in-your-project), was changed [years ago](https://reviews.llvm.org/D103044) to recommend this:
```
include_directories(${LLVM_INCLUDE_DIRS})
separate_arguments(LLVM_DEFINITIONS_LIST NATIVE_COMMAND ${LLVM_DEFINITIONS})
add_definitions(${LLVM_DEFINITIONS_LIST})
```
Rather than change all the callers across multiple projects, I'm wondering why downstream projects which embed LLVM need to make an `add_definitions` call at all.
The definitions in `LLVM_DEFINITIONS` are themselves derived from the `CMAKE_SOURCE_DIR` and thus wouldn't they already apply wherever `HandleLLVMOptions` is being invoked in the build? Can we just remove this recommendation/requirement?
In my setup, I typically embed LLVM by building-from-source in a super project using `add_subdirectory` to build LLVM. Invoking the recommended sequence above afterwards just causes a duplication of flags. Worse, if you have multiple other embedded out-of-tree MLIR/LLVM projects, and they also invoke the above sequence, then your flags will duplicate again. I actually observed on a very old system that this could cause some sort of overflow in the commands executed by cmake.
![]()
_______________________________________________
llvm-bugs mailing list
llvm-bugs@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-bugs
