Add option to build libomp library using Microsoft Visual C++ naming scheme.

[branh]

Update title.

[protze.joachim]

Added several questions inline

openmp/CMakeLists.txt

97

What does the 140 mean? Is this supposed to be the llvm release number?
I suggest to use a more descriptive name for this Cmake variable and not to encode a version number into the variable name.

openmp/runtime/CMakeLists.txt

359

Similar as above, what is 140?
Probably combine LLVM_VERSION_MAJOR and LLVM_VERSION_MINOR?

Why not make destinction Debug/non-Debug here?

openmp/runtime/src/CMakeLists.txt

162

CMAKE_BUILD_TYPE is case sensitive. Why make it case-insensitive here?

242–253

The IMP does not need the 140?

[jlpeyton]

openmp/runtime/src/CMakeLists.txt

162

To enable standalone builds as well, we can use DEBUG_BUILD variable (defined in openmp/runtime/CMakeLists.txt)

247

Same here, we can use DEBUG_BUILD

[branh]

Addressing review comments.

[branh]

openmp/runtime/CMakeLists.txt

359

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.

openmp/runtime/src/CMakeLists.txt

162

Every other use of CMAKE_BUILD_TYPE I found were case insensitive as well.

242–253

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.

[branh]

Squashing commits.

[branh]

openmp/runtime/src/CMakeLists.txt

162

@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?

[branh]

openmp/runtime/CMakeLists.txt

359

As for last question (why not make debug/non-debug distinction here), the dlls are named libomp140(d).dll whereas the libraries are named libomp(d).dll in the MSVC naming scheme. Making the name decision closer to where the name is actually used makes which name is used where clearer and helped ensure I didn't miss a name.

[jlpeyton]

openmp/runtime/src/CMakeLists.txt

162

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.

[branh]

openmp/runtime/src/CMakeLists.txt

162

@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?

[jlpeyton]

I only have one other suggestion. I think its fine to use uppercase_CMAKE_BUILD_TYPE since its defined everywhere. The DEBUG_BUILD (and friends) are useful as booleans for append_if() type macro/functions in CMake.

openmp/CMakeLists.txt

97

All the user-definable CMake variables in this CMakeLists.txt should be prefixed with OPENMP_, so I think the variable should be OPENMP_MSVC_NAME_SCHEME.

[branh]

Change name of option to include OPENMP

[branh]

Update name for option to include OPENMP

[jlpeyton]

LGTM

[branh]

My apologies, I don't have commit access. Could someone commit on my behalf? Thank you in advance.

openmp/CMakeLists.txt

 endif()
 
 option(OPENMP_ENABLE_OMPT_TOOLS "Enable building ompt based tools for OpenMP."
        ${ENABLE_OMPT_TOOLS})
 if (OPENMP_ENABLE_OMPT_TOOLS)
   add_subdirectory(tools)
 endif()
 
-option(OPENMP_BUILD_140 "Build dll with VC naming scheme." OFF)
+option(MSVC_NAME_SCHEME "Build dll with MSVC naming scheme." OFF)
 
 # Build documentation
 add_subdirectory(docs)
 
 # Now that we have seen all testsuites, create the check-openmp target.
 construct_check_openmp_target()

openmp/runtime/CMakeLists.txt

 set(LIBOMP_DEFAULT_LIB_NAME libomp)
 if(${PROFILE_LIBRARY})
   set(LIBOMP_DEFAULT_LIB_NAME ${LIBOMP_DEFAULT_LIB_NAME}prof)
 endif()
 if(${STUBS_LIBRARY})
   set(LIBOMP_DEFAULT_LIB_NAME ${LIBOMP_DEFAULT_LIB_NAME}stubs)
 endif()
 set(LIBOMP_LIB_NAME ${LIBOMP_DEFAULT_LIB_NAME} CACHE STRING "Base OMP library name")
-if (OPENMP_BUILD_140)
-  set(LIBOMP_LIB_NAME ${LIBOMP_LIB_NAME}140.${LIBOMP_ARCH})
+if (MSVC_NAME_SCHEME)
+  # MSVC_TOOLS_VERSION corresponds to the version of the VC++ toolset.
+  set(MSVC_TOOLS_VERSION 140)
+  set(LIBOMP_LIB_NAME ${LIBOMP_LIB_NAME}${MSVC_TOOLS_VERSION}.${LIBOMP_ARCH})
 endif()
 
 if(${LIBOMP_ENABLE_SHARED})
   set(LIBOMP_LIBRARY_SUFFIX ${CMAKE_SHARED_LIBRARY_SUFFIX})
   set(LIBOMP_LIBRARY_KIND SHARED)
   set(LIBOMP_INSTALL_KIND LIBRARY)
 else()
   set(LIBOMP_LIBRARY_SUFFIX ${CMAKE_STATIC_LIBRARY_SUFFIX})

openmp/runtime/src/CMakeLists.txt

else()
   add_llvm_library(omp ${LIBOMP_LIBRARY_KIND} ${LIBOMP_SOURCE_FILES} PARTIAL_SOURCES_INTENDED
     LINK_LIBS ${LIBOMP_CONFIGURED_LIBFLAGS} ${CMAKE_DL_LIBS}
     LINK_COMPONENTS Support
     )
   # libomp must be a C++ library such that it can link libLLVMSupport
   set(LIBOMP_LINKER_LANGUAGE CXX)
 endif()
 
-if(OPENMP_BUILD_140)
+if(MSVC_NAME_SCHEME)
   if(uppercase_CMAKE_BUILD_TYPE STREQUAL "DEBUG")
-    set(LIBOMP_PDB_NAME ${LIBOMP_DEFAULT_LIB_NAME}140d.${LIBOMP_ARCH})
+    set(LIBOMP_PDB_NAME ${LIBOMP_DEFAULT_LIB_NAME}${MSVC_TOOLS_VERSION}d.${LIBOMP_ARCH})
     set(LIBOMP_LIB_FILE ${LIBOMP_PDB_NAME}${LIBOMP_LIBRARY_SUFFIX})
   else()
-    # ${LIBOMP_LIB_NAME} is ${LIBOMP_DEFAULT_LIB_NAME}140.${LIBOMP_ARCH}
+    # ${LIBOMP_LIB_NAME} is ${LIBOMP_DEFAULT_LIB_NAME}${MSVC_TOOLS_VERSION}.${LIBOMP_ARCH}
     set(LIBOMP_PDB_NAME ${LIBOMP_LIB_NAME}) 
   endif()
 
   # in debug
   # LIBOMP_LIB_FILE should be LIBOMP_LIB_FILE_DBG = ${LIBOMP_LIB_NAME_DBG}${LIBOMP_LIBRARY_SUFFIX}
-  #           = ${LIBOMP_DEFAULT_LIB_NAME}140d.${LIBOMP_ARCH}${LIBOMP_LIBRARY_SUFFIX}
-  # COMPILE_PDB_NAME/PDB_NAME should be LIBOMP_LIB_NAME_DBG = ${LIBOMP_DEFAULT_LIB_NAME}140d.${LIBOMP_ARCH}
+  #           = ${LIBOMP_DEFAULT_LIB_NAME}${MSVC_TOOLS_VERSION}d.${LIBOMP_ARCH}${LIBOMP_LIBRARY_SUFFIX}
+  # COMPILE_PDB_NAME/PDB_NAME should be LIBOMP_LIB_NAME_DBG = ${LIBOMP_DEFAULT_LIB_NAME}${MSVC_TOOLS_VERSION}d.${LIBOMP_ARCH}
   set_target_properties(omp PROPERTIES
     PREFIX "" SUFFIX "" OUTPUT_NAME "${LIBOMP_LIB_FILE}"
     PDB_NAME "${LIBOMP_PDB_NAME}"
     COMPILE_PDB_NAME "${LIBOMP_PDB_NAME}"
     LINK_FLAGS "${LIBOMP_CONFIGURED_LDFLAGS}"
     LINKER_LANGUAGE ${LIBOMP_LINKER_LANGUAGE}
  )
 else()
if(WIN32)
   set_property(SOURCE z_Windows_NT-586_asm.asm APPEND_STRING PROPERTY COMPILE_FLAGS " ${LIBOMP_MASM_DEFINITIONS}")
   set_source_files_properties(thirdparty/ittnotify/ittnotify_static.cpp PROPERTIES COMPILE_DEFINITIONS "UNICODE")
 
   # Create Windows import library
   # the import library is "re-linked" to include kmp_import.cpp which prevents
   # linking of both Visual Studio OpenMP and newly built OpenMP
   set_source_files_properties(kmp_import.cpp PROPERTIES COMPILE_FLAGS "${LIBOMP_CONFIGURED_CXXFLAGS}")
 
   # 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(MSVC_NAME_SCHEME)
     if(uppercase_CMAKE_BUILD_TYPE STREQUAL "DEBUG")
       set(LIBOMP_IMP_LIB_FILE ${LIBOMP_DEFAULT_LIB_NAME}d${CMAKE_STATIC_LIBRARY_SUFFIX})
       set(LIBOMP_GENERATED_IMP_LIB_FILENAME ${LIBOMP_DEFAULT_LIB_NAME}d${LIBOMP_LIBRARY_SUFFIX}${CMAKE_STATIC_LIBRARY_SUFFIX})
     else()
       set(LIBOMP_IMP_LIB_FILE ${LIBOMP_DEFAULT_LIB_NAME}${CMAKE_IMPORT_LIBRARY_SUFFIX})
       set(LIBOMP_GENERATED_IMP_LIB_FILENAME ${LIBOMP_DEFAULT_LIB_NAME}${LIBOMP_LIBRARY_SUFFIX}${CMAKE_STATIC_LIBRARY_SUFFIX})
     endif()
   else()
     set(LIBOMP_IMP_LIB_FILE ${LIBOMP_LIB_NAME}${CMAKE_IMPORT_LIBRARY_SUFFIX})
     set(LIBOMP_GENERATED_IMP_LIB_FILENAME ${LIBOMP_LIB_FILE}${CMAKE_STATIC_LIBRARY_SUFFIX})
   endif()
   set_target_properties(omp PROPERTIES
     VERSION ${LIBOMP_VERSION_MAJOR}.${LIBOMP_VERSION_MINOR} # uses /version flag
     IMPORT_PREFIX "" IMPORT_SUFFIX "" # control generated import library name when building omp
     ARCHIVE_OUTPUT_NAME ${LIBOMP_GENERATED_IMP_LIB_FILENAME}