[PATCH] D79242: [MLIR] add dependencies for all tablegen targets on 'mlir-headers'

[Stephen Neuendorffer via Phabricator via llvm-commits]

stephenneuendorffer created this revision.
Herald added subscribers: llvm-commits, Kayjukh, frgossen, grosul1, bader, Joonsoo, liufengdb, aartbik, lucyrfox, mgester, arpith-jacob, nicolasvasilache, antiagainst, shauheen, jpienaar, rriddle, mehdi_amini, mgorny.
Herald added a reviewer: mravishankar.
Herald added a reviewer: antiagainst.
Herald added a reviewer: nicolasvasilache.
Herald added a reviewer: rriddle.
Herald added a project: LLVM.
stephenneuendorffer added a reviewer: vchuravy.
rriddle added inline comments.
stephenneuendorffer edited the summary of this revision.
stephenneuendorffer updated this revision to Diff 261503.
stephenneuendorffer updated this revision to Diff 261541.


================
Comment at: mlir/include/mlir/Dialect/LoopOps/CMakeLists.txt:7
 add_public_tablegen_target(MLIRLoopPassIncGen)
+add_dependencies(mlir-headers MLIRLoopPassIncGen)
 
----------------
Wonder if it would be easier to just have our own `add_public_mlir_tablegen_target` that does this automatically.


================
Comment at: mlir/lib/Analysis/CMakeLists.txt:5
   CallGraph.cpp
-  Dominance.cpp
   Liveness.cpp
----------------
These changes seem like they should be in the other revision?


================
Comment at: mlir/lib/Dialect/Linalg/Transforms/Fusion.cpp:14
 #include "PassDetail.h"
-#include "mlir/Analysis/Dominance.h"
+#include "mlir/IR/Dominance.h"
 #include "mlir/Dialect/Linalg/Analysis/DependenceAnalysis.h"
----------------
Were some of these changes supposed to be in the other revision?


================
Comment at: mlir/lib/Dialect/Linalg/Transforms/Fusion.cpp:14
 #include "PassDetail.h"
-#include "mlir/Analysis/Dominance.h"
+#include "mlir/IR/Dominance.h"
 #include "mlir/Dialect/Linalg/Analysis/DependenceAnalysis.h"
----------------
rriddle wrote:
> Were some of these changes supposed to be in the other revision?
Still unresolved?


================
Comment at: mlir/test/lib/Transforms/TestDominance.cpp:15
 
-#include "mlir/Analysis/Dominance.h"
+#include "mlir/IR/Dominance.h"
 #include "mlir/Pass/Pass.h"
----------------
Same here.


In cmake, dependencies on generated files require some sophistication in the build system.  At build time, files are parsed to determine which headers they depend on and these dependencies are injected into the build system.  This works well with ninja, but has some constraints with the makefile generator.  According to the cmake documentation, this only works reliably within the same directory.

This patch expands the usage of mlir-headers to include all generated headers and adds an mlir-generic-headers target which triggers generation of dialect-independent headers.  These targets are used to express dependencies on generated headers.  This is mostly handled in AddMLIR.cmake and only a few CMakeLists.txt files need to change.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D79242

Files:
  mlir/CMakeLists.txt
  mlir/cmake/modules/AddMLIR.cmake
  mlir/include/mlir/Dialect/Linalg/IR/CMakeLists.txt
  mlir/include/mlir/Dialect/LoopOps/CMakeLists.txt
  mlir/include/mlir/Dialect/SPIRV/CMakeLists.txt
  mlir/include/mlir/IR/CMakeLists.txt
  mlir/include/mlir/Interfaces/CMakeLists.txt
  mlir/lib/Analysis/CMakeLists.txt
  mlir/lib/Dialect/Linalg/IR/CMakeLists.txt
  mlir/lib/Parser/CMakeLists.txt

-------------- next part --------------
A non-text attachment was scrubbed...
Name: D79242.261541.patch
Type: text/x-patch
Size: 9960 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20200502/e662743c/attachment-0001.bin>