[Openmp-commits] [PATCH] D110354: Add option to build libomp library using Microsoft Visual C++ naming scheme.

[Bran Hagger via Phabricator via Openmp-commits]

branh added inline comments.


================
Comment at: openmp/runtime/src/CMakeLists.txt:162
+if(OPENMP_BUILD_140)
+  if(uppercase_CMAKE_BUILD_TYPE STREQUAL "DEBUG")
+    set(LIBOMP_PDB_NAME ${LIBOMP_DEFAULT_LIB_NAME}140d.${LIBOMP_ARCH})
----------------
jlpeyton wrote:
> branh wrote:
> > branh wrote:
> > > jlpeyton wrote:
> > > > protze.joachim wrote:
> > > > > CMAKE_BUILD_TYPE is case sensitive. Why make it case-insensitive here?
> > > > To enable standalone builds as well, we can use `DEBUG_BUILD` variable (defined in openmp/runtime/CMakeLists.txt)
> > > Every other use of CMAKE_BUILD_TYPE I found were case insensitive as well.
> > @protze.joachim @jlpeyton , I'm still confused by the comments about DEBUG_BUILD. As far as I can tell, a case-insensitive version of CMAKE_BUILD_TYPE (sometimes upper case, sometimes lower case) is the method used to determine build type in every CMakeLists.txt. DEBUG_BUILD is set in a CMakeLists.txt file, but it is only used for header files and setting compiler flags.
> > 
> > Is there a plan to switch to using DEBUG_BUILD in CMakeLists.txt files instead of checking CMAKE_BUILD_TYPE at some point in the future?
> I've created https://reviews.llvm.org/D112951 to alleviate any concerns I would have with your approach for `CMAKE_BUILD_TYPE`. There are two ways to build libomp
> 1) Standalone build which only builds libomp (and not any other part of LLVM)
> 2) As a part of LLVM. I'm sure this is the grand majority of builds but don't have any statistics on it.
> 
> The standalone build doesn't know what `uppercase_CMAKE_BUILD_TYPE` is since it is defined in llvm/CMakeLists.txt. D112951 will add `uppercase_CMAKE_BUILD_TYPE` for the standalone build so there won't be any issue with your approach.
@jlpeyton That makes sense. Thank you for clearing that up.

Given your proposed changeset, are there any more changes that you think should be made to this changeset before it is accepted? Should I still modify this changeset to use DEBUG_BUILD?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D110354