[PATCH] D125010: [MLIR] Fix build with make

[River Riddle via Phabricator via llvm-commits]

rriddle added a comment.

In D125010#3494133 <https://reviews.llvm.org/D125010#3494133>, @nikic wrote:

> In D125010#3494007 <https://reviews.llvm.org/D125010#3494007>, @marbre wrote:
>
>> I am currently out-of-office and don‘t have any access to my build machines. Therefore, I cannot test and confirm that the patch doesn‘t have unintended side effects. However, there is definitely a need  to add a dependency to the tablgen generated header files. Sorry for missing this as part of https://reviews.llvm.org/D124851. I would suggest to take a look how this is done for MLIR dialects, where explicits deps to the generated targets are passed to `add_mlir_dialect_library` like in https://github.com/llvm/llvm-project/blob/04b419048955fc33718ba97e79f3558b6a27830e/mlir/lib/Dialect/EmitC/IR/CMakeLists.txt#L7-L9. It might not be necessary to touch AddLLVM and I assume the deps should rather be on the `*IncGen` target,
>
> I did try just adding MLIRBuiltinAttributeInterfacesIncGen under DEPENDS first, but this ultimately doesn't do anything useful without the change to AddLLVM, because the the intermediate objlib target (for non-ninja generators) essentially means that mlir-pdll depends on both mlir-pdll.cpp.o and MLIRBuiltinAttributeInterfacesIncGen and these are not sequenced (what we need is for mlir-pdll.cpp.o to depend on MLIRBuiltinAttributeInterfacesIncGen).
>
> As to depending directly on `*IncGen` targets, I can change it in that direction -- my thought here was that the exact dependencies of the headers are something of an implementation detail. But if other areas already hardcode these, then also doing that here makes sense to me as well.

Your thought is right here, the IncGen should already be provided via MLIRIR. Most IncGen shouldn't be depended on directly.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D125010/new/

https://reviews.llvm.org/D125010