Skip to content

Conversation

@christopherbate
Copy link
Contributor

Fixes the following two issues from the Standalone project's
CMakeLists.txt:

  • LLVM_DEFINITIONS is formatted as string by LLVMConfig.cmake in a
    manner which will cause unexpected results if the first definition
    in is of the form -Dkey=value. It must be preprocessed before passing
    it to add_definitions, see the recommendation in the LLVM CMake
    documentation.

  • The HandleLLVMOptions script resets LLVM_DEFINITIONS to the definitions
    associated with the top-level source directory, so if the project is
    top level, using LLVM_DEFINITIONS after invoking HandleLLVMOptions
    is circular. Invoke HandleLLVMOptions only if the project is top-level
    and only after using LLVM_DEFINITIONS.

Fixes the following two issues from the Standalone project's
CMakeLists.txt:

- LLVM_DEFINITIONS is formatted as string by `LLVMConfig.cmake` in a
  manner which will cause unexpected results if the first definition
  in is of the form `-Dkey=value`. It must be preprocessed before passing
  it to `add_definitions`, see the recommendation in the LLVM CMake
  [documentation](https://llvm.org/docs/CMake.html#embedding-llvm-in-your-project).

- The `HandleLLVMOptions` script resets `LLVM_DEFINITIONS` to the definitions
  associated with the top-level source directory, so if the project is
  top level, using `LLVM_DEFINITIONS` after invoking `HandleLLVMOptions`
  is circular. Invoke `HandleLLVMOptions` only if the project is top-level
  and only after using `LLVM_DEFINITIONS`.
@llvmbot llvmbot added the mlir label Feb 18, 2025
@llvmbot
Copy link
Member

llvmbot commented Feb 18, 2025

@llvm/pr-subscribers-mlir

Author: Christopher Bate (christopherbate)

Changes

Fixes the following two issues from the Standalone project's
CMakeLists.txt:

  • LLVM_DEFINITIONS is formatted as string by LLVMConfig.cmake in a
    manner which will cause unexpected results if the first definition
    in is of the form -Dkey=value. It must be preprocessed before passing
    it to add_definitions, see the recommendation in the LLVM CMake
    documentation.

  • The HandleLLVMOptions script resets LLVM_DEFINITIONS to the definitions
    associated with the top-level source directory, so if the project is
    top level, using LLVM_DEFINITIONS after invoking HandleLLVMOptions
    is circular. Invoke HandleLLVMOptions only if the project is top-level
    and only after using LLVM_DEFINITIONS.


Full diff: https://github.com/llvm/llvm-project/pull/127716.diff

1 Files Affected:

  • (modified) mlir/examples/standalone/CMakeLists.txt (+6-1)
diff --git a/mlir/examples/standalone/CMakeLists.txt b/mlir/examples/standalone/CMakeLists.txt
index 038242ba1437a..34167dcc7985a 100644
--- a/mlir/examples/standalone/CMakeLists.txt
+++ b/mlir/examples/standalone/CMakeLists.txt
@@ -18,6 +18,12 @@ if(CMAKE_SOURCE_DIR STREQUAL CMAKE_CURRENT_SOURCE_DIR)
   list(APPEND CMAKE_MODULE_PATH "${MLIR_CMAKE_DIR}")
   list(APPEND CMAKE_MODULE_PATH "${LLVM_CMAKE_DIR}")
 
+  # This must go before 'HandleLLVMOptions', otherwise
+  # it has no effect. It shouldn't be required when
+  # building via the external projects mechanism.
+  separate_arguments(LLVM_DEFINITIONS_LIST NATIVE_COMMAND ${LLVM_DEFINITIONS})
+  add_definitions(${LLVM_DEFINITIONS_LIST})
+
   include(TableGen)
   include(AddLLVM)
   include(AddMLIR)
@@ -42,7 +48,6 @@ include_directories(${MLIR_INCLUDE_DIRS})
 include_directories(${STANDALONE_SOURCE_DIR}/include)
 include_directories(${STANDALONE_BINARY_DIR}/include)
 link_directories(${LLVM_BUILD_LIBRARY_DIR})
-add_definitions(${LLVM_DEFINITIONS})
 
 add_subdirectory(include)
 add_subdirectory(lib)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants