joker-eph wrote: It's closed, but not the same, I came to this by asking the agent to run the full configuration matrix and diagnose each cases (including clients calling `find_package(MLIR)` before and after `find_package(Clang)`). So there is more than just error messages and the behavior is quite different here.
Here is the detailed report from the agent: Both changes fix the same symptom: with `CLANG_ENABLE_CIR=ON` and `mlir` in `LLVM_ENABLE_PROJECTS`, an external `find_package(Clang)` fails with "The following imported targets are referenced, but are missing: MLIRIR MLIRPass ..." unless the consumer calls `find_package(MLIR)` first. They solve it with opposite strategies, and the behavior differs materially when MLIR is a full project. The results below were measured by configuring an LLVM tree (`clang;mlir`, `CLANG_ENABLPE_CIR=ON`, CMake 4.3.0-rc2) and running the same configure-only consumer project against it. ## The approaches ### PR #226763: bundle - Drops the dependency-only guard in `clang/CMakeLists.txt` and promotes the MLIR closure reachable from Clang's exports into `ClangTargets.cmake` even when `mlir` is in `LLVM_ENABLE_PROJECTS`, via a new `DEPENDENCY_PROJECTS` argument to `llvm_install_dependency_target_closure`. - `ClangConfig.cmake` never calls `find_package(MLIR)`. - MLIR headers are only installed by Clang in dependency-only mode. - PR #226763 's first revision instead did a REQUIRED `find_package(MLIR)` in `ClangConfig.cmake`; the review thread moved it to bundling. ### This PR: delegate - Clang keeps MLIR-owned targets out of its export set when MLIR is a full project. Bundling stays in place for the dependency-only mode. - `ClangConfig.cmake` performs an optional, quiet `find_package(MLIR)` (EXACT version, hinted at the sibling package dir) before including `ClangTargets.cmake`, and appends a ClangIR/MLIR hint to the not-found message if targets end up missing. - `CMAKE_FIND_REQUIRED` is cleared and restored around the lookup so a direct `include(ClangConfig)` keeps MLIR optional. - The dependency-only build-tree `MLIRConfig.cmake` reports MLIR as not found with an explanation instead of succeeding with zero targets. - Release note added under "Changes to building LLVM". ## Behavior with `clang;mlir` and ClangIR on | Consumer does | PR #226763 | This PR | |---|---|---| | `find_package(Clang)` only | Works. MLIR package not loaded, no `MLIR_*` variables | Works. MLIR package loaded, 524 MLIR targets visible | | `find_package(MLIR)` then `find_package(Clang)` | Fatal: "Some (but not all) targets in this export set were already defined" | Works | | `find_package(Clang)` then `find_package(MLIR)` | `MLIR_FOUND=1` but 426 of 524 exported MLIR targets missing (e.g. `MLIRAsyncDialect`). `MLIRConfig` sees `MLIRSupport` already defined and skips `MLIRTargets.cmake` | Works, all 524 targets present | | `find_package(Clang)` with no MLIR package available | Works, Clang carries what it needs | Clang not found, targets-file message plus the ClangIR/MLIR hint | | `include(ClangConfig)` with `CMAKE_FIND_REQUIRED=ON` | Works (no nested lookup) | Works, variable preserved | The third row is the dangerous one. PR #226763 produces a silently truncated MLIR SDK rather than an error, and a consumer only notices at link time or when a dialect target is missing. ## Install tree differences - **Ownership of the MLIR targets reachable from Clang (103 in this build).** Under PR #226763 they are installed twice: once by MLIR's own rules into the MLIR export set, and again by Clang's `clang-dependencies` component into the Clang export set. Both `ClangTargets.cmake` and `MLIRTargets.cmake` then define them, which is the source of the ordering failures above. Under this PR they are installed once, by MLIR, and appear only in `MLIRTargets.cmake`; `ClangTargets.cmake` references them without defining them. The `MLIRCIR*` libraries are Clang targets and stay in Clang's export set in both. - **Dependency-only mode** (`clang` enabled, `mlir` pulled in implicitly) is unchanged by both: there is no MLIR package, and Clang installs the reachable MLIR closure into its own export set. - **Distributions that ship Clang without MLIR** import fine under PR #226763, since Clang is self-contained. The branch handles that case by keeping the lookup optional and letting the targets file report any missing targets. ## Other differences - PR #226763 does not touch the dependency-only `MLIRConfig.cmake`, so a CIR-only build tree still reports `MLIR_FOUND=1` with zero targets. This PR makes it report not found. - Under PR #226763, `find_package(Clang)` never exposes MLIR variables such as `MLIR_TABLEGEN_EXE` or `MLIR_DIR`. Under this PR it does, matching what `find_package(Flang)` already does for its Clang and MLIR dependencies. ## Net assessment PR #226763 's approach only works if consumers never call `find_package(MLIR)` themselves. Any downstream with its own dialects, which is exactly the Enzyme-CIR use case cited in the PR thread, needs the MLIR package and hits one of the two ordering failures. This PR makes the order irrelevant at the cost of loading the MLIR package into the consumer, which is the contract Flang already has. PR #226763 's bundling remains the right design for the dependency-only mode, and this PR keeps it there. https://github.com/llvm/llvm-project/pull/228366 _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
