[PATCH] D82659: Fix missing build dependency on omp_gen.

[Valentin Clement via Phabricator via llvm-commits]

clementval added a comment.

In D82659#2123127 <https://reviews.llvm.org/D82659#2123127>, @simon_tatham wrote:

> > Cannot you just add a depends for your failing case?
>
> Perhaps. But the problem is that `OMP.h.inc` ends up being included from a lot of places – for example, anything that includes `clang/AST/AST.h` ultimately needs it. So the first problem in adding the missing dependencies on `omp_gen` is to find out where they all are, and that's not trivial.
>
> In order to identify //all// the missing dependencies, I just wrote a small Python script that re-processes the output of `ninja -t deps` and `ninja -t commands` to find object files that can't be built without `OMP.h.inc` but don't have a formal dependency on a command that builds it.
>
> That script showed that there are objects in `clang/lib/CodeGen` and `clang/lib/Frontend` that don't have this dependency, and also the extra awkward `clang/examples/PrintFunctionNames`, for which it's not even obvious //where// the extra dependency should be inserted.
>
> But even if we put those in, it will guarantee that the build works //now//, but not that it will carry on working when someone adds a `#include` in a source directory that doesn't depend on `omp_gen`. So I don't think this really deserves the word "just"!


Ok, the word "just" is not a perfect fit here. Anyway, if `omp_gen` can be set up in a similar manner than `intrinsics_gen` it would probably be better than merging `omp_gen` into `intrinsics_gen` since they are different enough to have a separated target. `intrinsics_gen` is added as a dependency in a lot of place so I'm not sure it would solve the problem to add dependencies where needed.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D82659