Skip to content

fix(cmake): remove redundant add_definitions(${LLVM_DEFINITIONS})#3021

Merged
fifield merged 4 commits intoXilinx:mainfrom
FIM43-Redeye:fix/gcc-compile-defs
Apr 21, 2026
Merged

fix(cmake): remove redundant add_definitions(${LLVM_DEFINITIONS})#3021
fifield merged 4 commits intoXilinx:mainfrom
FIM43-Redeye:fix/gcc-compile-defs

Conversation

@FIM43-Redeye
Copy link
Copy Markdown
Contributor

@FIM43-Redeye FIM43-Redeye commented Apr 7, 2026

Summary

  • Remove redundant add_definitions(${LLVM_DEFINITIONS}) from all six CMakeLists.txt files that also include(HandleLLVMOptions)
  • HandleLLVMOptions already adds every flag individually via add_compile_definitions(), making these calls purely duplicative
  • The redundant calls caused garbled -D flags with GCC because LLVM_DEFINITIONS is a space-separated string, not a CMake list

Fixes #2965.

Test plan

  • Verified target_model, target_model_rtti, and register_database CppTests build and pass with Clang after removal
  • CI: GCC compile matrix should now pass cleanly

Generated using Claude Code.

HandleLLVMOptions already adds all LLVM compile definitions individually
via add_compile_definitions(). The add_definitions(${LLVM_DEFINITIONS})
calls were redundant and caused garbled -D flags with GCC because
LLVM_DEFINITIONS is a space-separated string, not a CMake list.

Fixes Xilinx#2965.

Generated using Claude Code.
@FIM43-Redeye FIM43-Redeye marked this pull request as ready for review April 13, 2026 17:40
Copilot AI review requested due to automatic review settings April 13, 2026 17:40
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR removes redundant add_definitions(${LLVM_DEFINITIONS}) calls from CMake subprojects that already include(HandleLLVMOptions), avoiding duplicated/garbled -D definitions that break GCC builds (issue #2965).

Changes:

  • Removed add_definitions(${LLVM_DEFINITIONS}) from six CMakeLists.txt files that already include HandleLLVMOptions.
  • Ensures LLVM compile definitions are applied only via LLVM’s CMake machinery, preventing malformed preprocessor definitions with GCC.

Reviewed changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated no comments.

Show a summary per file
File Description
CMakeLists.txt Drops redundant LLVM compile definitions in the top-level build configuration.
test/CMakeLists.txt Drops redundant LLVM compile definitions for test configuration (affects CppTests build).
utils/mlir_aie_wheels/python_bindings/CMakeLists.txt Drops redundant LLVM compile definitions for wheel/python bindings build.
programming_guide/CMakeLists.txt Drops redundant LLVM compile definitions for programming guide build/tests.
programming_examples/CMakeLists.txt Drops redundant LLVM compile definitions for examples build/tests.
mlir_exercises/CMakeLists.txt Drops redundant LLVM compile definitions for exercises build/tests.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@jgmelber jgmelber added this pull request to the merge queue Apr 14, 2026
@github-merge-queue github-merge-queue Bot removed this pull request from the merge queue due to no response for status checks Apr 14, 2026
@FIM43-Redeye
Copy link
Copy Markdown
Contributor Author

CI checks out,but the queue bot kicked it out on Apr 14. Still seems okay, though. Anything else to take care of before merge?

@fifield fifield added this pull request to the merge queue Apr 21, 2026
Merged via the queue into Xilinx:main with commit 5fa7c23 Apr 21, 2026
71 checks passed
@FIM43-Redeye FIM43-Redeye deleted the fix/gcc-compile-defs branch April 21, 2026 18:17
joeldushouyu pushed a commit to joeldushouyu/mlir-aie that referenced this pull request Apr 24, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

CppTests fail to build with GCC due to duplicate compile definitions

4 participants