[PATCH] D78771: Add dependency edges between generated headers and users

[Stephen Neuendorffer via Phabricator via llvm-commits]

stephenneuendorffer requested changes to this revision.
stephenneuendorffer added inline comments.
This revision now requires changes to proceed.


================
Comment at: mlir/lib/Analysis/CMakeLists.txt:24
+  DEPENDS
+  MLIRCallInterfacesIncGen
+  MLIRLoopLikeInterfaceIncGen
----------------
mehdi_amini wrote:
> vchuravy wrote:
> > rriddle wrote:
> > > This seems really annoying if necessary. There is already an explicit library for the dependency, e.g. MLIRCallInterfaces, this seems redundant and exposes things that shouldn't necessarily need to be exposed.
> > Yeah it is really annoying... 
> > 
> > I had to add these for https://reviews.llvm.org/D78773 so it my be an issue with that PR.
> > 
> > To reproduce on D78773, this should be enough
> > ```
> > cmake -G Ninja -DCMAKE_BUILD_TYPE=Release -DLLVM_TARGETS_TO_BUILD=host -DLLVM_ENABLE_PROJECTS=mlir -DLLVM_BUILD_LLVM_DYLIB:BOOL=ON ../llvm
> > ninja
> > ```
> @rriddle as far as I understand we need the `DEPENDS` directive to express a that target must be built (here generated headers) before you build the source for this library, while the `target_link_libraries`dependency only induces a link-time dependency.
> That said different version of CMake (or different environments?) seem to behave differently on this, in the past @stephenneuendorffer observed different dependency than me.
> 
> For example right now for me `ninja clean && ninja tools/mlir/lib/Analysis/CMakeFiles/MLIRAnalysis.dir/CallGraph.cpp.o -nv | grep CallInterfaces` shows that these dependency are already there.
> 
see comments on 78773.  I think there's a simple way to fix it.  The problem is actually that cmake needs special handling for dependencies on with generated files. (namely the .inc files)  After the files are generated, it can see them and compute dependencies correctly using clang.  However, before the files are generated it needs some extra implicit dependency to bootstrap the whole process.   These extra dependencies seem to be a special case in cmake:  only for PUBLIC dependencies which are library dependencies.  D78771 partially breaks that because it enables object libraries, which results in another set of not-quite library targets which generate .o files for each .cpp file.  These are included as regular sources, not library dependencies.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D78771