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

[Valentin Clement via Phabricator via llvm-commits]

clementval added a comment.

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

> Unfortunately, I had to revert this because it caused a buildbot failure: rG39ea5d74b283d5a42f34b856d22bfaf806a1c907 <https://reviews.llvm.org/rG39ea5d74b283d5a42f34b856d22bfaf806a1c907>
>
> That puzzled me for a while, but I think I see the problem: `llvm/cmake/modules/CMakeLists.txt` calls `configure_file` to write out a fresh set of CMake files for modules to use, in which the main CMake system's definition of `LLVM_COMMON_DEPENDS` is exported, without necessarily exporting all the targets included in it.
>
> `modules/CMakeLists.txt` includes code to remove the existing phony target `intrinsics_gen` from that variable before exporting it. So one fix would be to add `omp_gen` to the same code.
>
> However, @clementval, do you think it would be a better idea to make your OpenMP tablegen command part of `intrinsics_gen`, along with all the rest of them, and remove `omp_gen` completely? That would make it work just like everything else. But perhaps you know more than I do about why that wouldn't work?




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

> Unfortunately, I had to revert this because it caused a buildbot failure: rG39ea5d74b283d5a42f34b856d22bfaf806a1c907 <https://reviews.llvm.org/rG39ea5d74b283d5a42f34b856d22bfaf806a1c907>
>
> That puzzled me for a while, but I think I see the problem: `llvm/cmake/modules/CMakeLists.txt` calls `configure_file` to write out a fresh set of CMake files for modules to use, in which the main CMake system's definition of `LLVM_COMMON_DEPENDS` is exported, without necessarily exporting all the targets included in it.
>
> `modules/CMakeLists.txt` includes code to remove the existing phony target `intrinsics_gen` from that variable before exporting it. So one fix would be to add `omp_gen` to the same code.
>
> However, @clementval, do you think it would be a better idea to make your OpenMP tablegen command part of `intrinsics_gen`, along with all the rest of them, and remove `omp_gen` completely? That would make it work just like everything else. But perhaps you know more than I do about why that wouldn't work?


Since `intrinsic_gen` is used in many places, I'm not sure we want to pollute it with the `omp_gen`. Cannot you just add a depends for your failing case? I see many things depending on `intrinsic_gen` so it shouldn't harm to add a depend omp_gen where needed.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D82659