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

[Bran Hagger via Phabricator via Openmp-commits]

branh marked an inline comment as done.
branh added inline comments.


================
Comment at: openmp/runtime/CMakeLists.txt:359
+if (OPENMP_BUILD_140)
+  set(LIBOMP_LIB_NAME ${LIBOMP_LIB_NAME}140.${LIBOMP_ARCH})
+endif()
----------------
protze.joachim wrote:
> Similar as above, what is 140?
> Probably combine LLVM_VERSION_MAJOR and LLVM_VERSION_MINOR?
> 
> Why not make destinction Debug/non-Debug here?
The version number corresponds to the toolset the dll is built for. The last time the Visual C++ toolset broke binary compatibility was Visual Studio 14, so the version number is 14.0.


================
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:
> 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.


================
Comment at: openmp/runtime/src/CMakeLists.txt:242-253
+  # On debug LIBOMP_IMP_LIB_FILE should be LIBOMP_IMP_LIB_FILE_DBG = ${LIBOMP_DEFAULT_LIB_NAME_DBG}${CMAKE_STATIC_LIBRARY_SUFFIX}
+  # 									${LIBOMP_DEFAULT_LIB_NAME}d${CMAKE_STATIC_LIBRARY_SUFFIX}
+  # and the ARCHIVE_OUTPUT_NAME of ompdbg should be ${LIBOMP_DEFAULT_LIB_NAME_DBG}${LIBOMP_LIBRARY_SUFFIX}${CMAKE_STATIC_LIBRARY_SUFFIX} 
+
+  if(OPENMP_BUILD_140)
+    if(uppercase_CMAKE_BUILD_TYPE STREQUAL "DEBUG")
+      set(LIBOMP_IMP_LIB_FILE ${LIBOMP_DEFAULT_LIB_NAME}d${CMAKE_STATIC_LIBRARY_SUFFIX})
----------------
protze.joachim wrote:
> The IMP does not need the 140?
Only dlls need the 140. The version # is part of the dll name to allow different versions of the same dll to exist on the machine side-by-side without interfering with each other at runtime/load time. Library files are only used at build time, so they don't need the version number.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D110354