[Openmp-commits] [PATCH] D102587: [OpenMP] Refine CMake code for libomptarget

[Shilei Tian via Phabricator via Openmp-commits]

tianshilei1992 added inline comments.


================
Comment at: openmp/libomptarget/CMakeLists.txt:47
+  libomptarget_say("Building libomptarget requires LLVM 12.0+ components. Will not build libomptarget.")
+  return()
 endif()
----------------
ye-luo wrote:
> Could  you unset cached variable OPENMP_ENABLE_LIBOMPTARGET (in a proper place)? Just to keep CMakeCached consistent.
> 
> Since OPENMP_ENABLE_LIBOMPTARGET is not in the current CMakeLists.txt, the unset probably needs to be at openmp/CMakeLists.txt
> and the current CMakeLists.txt needs to set a return value signalling libomptarget not being built.
I feel that would be not a good direction. IMO, `OPENMP_ENABLE_LIBOMPTARGET` is just an option which presumably should be set by users. Its only functionality is to tell CMake whether we need to involve `libomptarget`. As for whether `libomptarget` can be built successfully, it's out of its control. Even if due to some reasons it is not built, as long as users don't set it to OFF, `libomptarget` still needs to be involved. For example, after users revolve issues found, it just needs to reconfigure the project instead set the variable to ON explicitly.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D102587