[Openmp-commits] [PATCH] D111983: [libomptarget][DeviceRTL] Generalise and simplify cmakelists

[Michael Kruse via Phabricator via Openmp-commits]

Meinersbur added a comment.

Missed a comment at the previous Submit button press.



================
Comment at: openmp/libomptarget/DeviceRTL/CMakeLists.txt:142
 add_custom_target(omptarget-new-nvptx-bc)
 add_dependencies(omptarget-new-nvptx-bc opt llvm-link)
 
----------------
JonChesterfield wrote:
> ^ this looks wrong, can anyone confirm?
> 
> I think it says that the target of all bitcode libraries depends on opt and llvm-link, however each of the libraries already depends on opt and llvm-link, using code that carefully only adds that dependency if they're part of the build.
> 
> If I'm reading this right, this line:
> > add_dependencies(omptarget-new-nvptx-bc opt llvm-link)
> is redundant in the best case and will break the build in the worst case
Redundant dependencies have no consequences, but forgetting them may result in non-deterministic failures.

I don't see how it could break the build as long as all targets exist. opt and llvm-link targets exist in both build configs, LLVM_ENABLE_PROJECTS and LLVM_ENABLE_RUNTIMES.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D111983