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

[Jonathan Peyton via Phabricator via Openmp-commits]

jlpeyton 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})
----------------
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.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D110354