[Mlir-commits] [mlir] [mlir, CMake] Depend only on the generated headers included across dialects (PR #221829)

[Mehdi Amini]

joker-eph wrote:

I asked an agent to investigate my concerns, here is the result:

### Blocking correctness failure

  At PR head 646389ac80fa, from a clean generated-header state:

  ninja MLIRArmNeonDialect -j32

  fails with:

  Arith.h:29:10: fatal error:
  'mlir/Dialect/Arith/IR/ArithOpsDialect.h.inc' file not found

  The dependency chain is:

  ArmNeonDialect.cpp
    → VectorOps.h
      → Arith.h
        → ArithOpsDialect.h.inc

  The PR promotes MLIRVectorIncGen into mlir-generic-headers, but not the transitively required MLIRArithOpsIncGen. See ArmNeonDialect.cpp, VectorOps.h, and Arith.h.

  A second clean failure occurs for MLIRLinalgDialect:

  LinalgDialect.cpp
    → ShardingInterface.h
      → ShardOps.h
        → ShardEnums.h.inc

  MLIRShardingInterfaceIncGen is declared, but MLIRShardIncGen is not ordered. See LinalgDialect.cpp and ShardingInterface.h.

  A broader scan found at least 12 affected objects across nine MLIR targets, including MLIRArmNeonDialect, MLIRLinalgDialect, MLIRTosaDialect, MLIRShapeOpsTransforms, and several AMDGPU/SPIR-V conversions.

  ### Why the reported build passes

  My clean clang+CIR build completed all 4,360 edges, while the isolated library build failed. The broad build happens to schedule unrelated TableGen work early enough; once compilation succeeds, Ninja’s depfile database can mask the missing
  edge on subsequent builds. Thus successful CI and ninja clang do not establish correctness.

  ninja -t missingdeps is indeed the feature you remembered. On the two reproductions it reported:

  Error: There are 2 missing dependency paths.
  2 targets had depfile dependencies on 6 distinct generated inputs
  without a non-depfile dep path to the generator.

  Ninja documents that this tool specifically detects generated inputs without an explicit or order-only path and warns about clean-build flakiness. It has been available since 1.11. Ninja manual

  ### Recommendation

  Restore the blanket dependency removed in AddMLIR.cmake until:

  1. Every affected target has an explicit generated-header dependency.
  2. Representative leaf targets build from an empty output tree.
  3. A populated build passes ninja -t missingdeps with no new findings.

  Longer-term, CMake 3.28’s public/interface header file sets and CMP0154 provide a better ownership model, but LLVM currently supports CMake 3.20, so adopting that requires a compatibility plan. CMake CMP0154

  There is also a secondary inconsistency: the PR makes mlir-generic-headers contain whole dialect generators while it remains documented as dialect-independent.


https://github.com/llvm/llvm-project/pull/221829