[Openmp-commits] [PATCH] D40081: [CMake] Refactor common settings and flags

[Jonas Hahnfeld via Phabricator via Openmp-commits]

Hahnfeld added inline comments.


================
Comment at: libomptarget/CMakeLists.txt:47-49
 # We need C++11 support.
-if(LIBOMPTARGET_HAVE_STD_CPP11_FLAG)
+if(OPENMP_HAVE_STD_CPP11_FLAG)
   
----------------
jlpeyton wrote:
> While we are cleaning up... I think this would be cleaner if all prerequisites were checked here and an error issued if one is not met.  Currently there seems to be only one (need -std=c++11 flag)  I don't think it would hurt to check for users trying to build on mac or windows here either and issue a similar message, because that would mean they tried to improperly override OPENMP_ENABLE_LIBOMPTARGET
> 
> ```
> # Check prerequisites for building libomptarget
> if (NOT OPENMP_HAVE_STD_CPP11_FLAG)
>   libomptarget_error_say("Requested build of libomptarget with no -std=c++11 flag")
> endif()
> ```
> 
> You could also add OPENMP_HAVE_STD_CPP11_FLAG into the logic which determines the default ENABLE_LIBOMPTARGET inside the top-level CMakeLists.txt file.
> 
> If a user somehow finds them self inside the libomptarget/CMakeLists.txt file without -std=c++11 flag support.  I think its better for CMake to scream at the user during config time rather than for a status message to get lost in a sea of CMake output, because then the user builds all of LLVM and they scratch their heads trying to find libomptarget.
I disagree: If the user explicitly tells the build system to build libomptarget, it's the user's fault.

Regarding C++11: Doesn't libomp also depend on that standard? Maybe we should just error out completely in the top-level `CMakeLists.txt`, I think LLVM requires a C++11-capable compiler anyway...


https://reviews.llvm.org/D40081