[openmp] Fix building for mingw targets after import library changes

[mstorsjo]

Summary

06d9bf5e64d472db5485815d9c3f70631064bb25 (https://reviews.llvm.org/D143431)
did a large restructuring of how the import library is created;
previously, a second step to tweak the import library was only
done for MSVC style targets, but after this commit, that logic
was applied for mingw targets too.

Since LIBOMP_GENERATED_IMP_LIB_FILENAME and LIBOMP_IMP_LIB_FILE
are equal on mingw targets (both are "libomp.dll.a", while they
are "libomp.dll.lib" and "libomp.lib" for MSVC targets), this caused
a conflict, with errors like this:

ninja: error: build.ninja:875: multiple rules generate runtime/src/libomp.dll.a [-w dupbuild=err]

Skip the logic with a second step to recreate the import library
for mingw targets. The MSVC specific logic for this relies on
running the static archiver with CMAKE_LINK_DEF_FILE_FLAG, which
with MS lib.exe (and llvm-lib) ignore the input object files and
just generates an import library - but mingw style tools don't
support this mode of operation. (By attemptinig the same, mingw tools
would generate a static library with the def file as one member.)
With mingw tools, the same can be achieved by invoking the dlltool
executable instead.

Instead of adding alternative logic for invoking dlltool, just skip
the second import library step, since neither GNU nor LLVM mingw
tools actually generate import libraries that link by ordinal - so
there's no need for a second import library.

[mstorsjo]

Does anyone want to comment on this one, or should I just go ahead and revert D143431 to unbreak my continuous builds, and we can fold this into an updated version of that patch?

[tianshilei1992]

I don't know too much on Windows, but it looks reasonable to me. Better to wait for @jlpeyton and @natgla.

[vadikp-intel]

The patch looks OK to fix the mingw build for now. We have work in flight to address a few issues brought up here, e.g. kmp_import, the need to build the second set of sources and .def support outside of the MSVC compatible toolsets (e.g. llvm-lib) that may further touch on this particular script.

[mstorsjo]

In D143992#4127289, @vadikp-intel wrote:

The patch looks OK to fix the mingw build for now.

Ok, thanks! Would you mind marking it as approved, before I go ahead and commit it?

We have work in flight to address a few issues brought up here, e.g. kmp_import, the need to build the second set of sources and .def support outside of the MSVC compatible toolsets (e.g. llvm-lib) that may further touch on this particular script.

Ok, that sounds good to hear! Feel free to subscribe me to such patches, and I'll try to chime in if I have something to add on it.

[vadikp-intel]

Looks good. I will include Martin in the subsequent reviews here.

openmp/runtime/cmake/LibompMicroTests.cmake

 #  - Fails if sees a dependence not listed in td_exp variable below
 #  - Program dependencies: perl, (unix)readelf, (mac)otool[64], (windows)link.exe
 #  - Available for Unix,Mac,Windows, Intel(R) MIC Architecture dynamic builds and Windows
 #    static builds. Not available otherwise.
 
 # get library location
 if(WIN32)
   get_target_property(LIBOMP_OUTPUT_DIRECTORY omp RUNTIME_OUTPUT_DIRECTORY)
-  get_target_property(LIBOMPIMP_OUTPUT_DIRECTORY ompimp ARCHIVE_OUTPUT_DIRECTORY)
+  get_target_property(LIBOMPIMP_OUTPUT_DIRECTORY ${LIBOMP_IMP_LIB_TARGET} ARCHIVE_OUTPUT_DIRECTORY)
   if(NOT LIBOMPIMP_OUTPUT_DIRECTORY)
     set(LIBOMPIMP_OUTPUT_DIRECTORY ${CMAKE_CURRENT_BINARY_DIR})
   endif()
 else()
   get_target_property(LIBOMP_OUTPUT_DIRECTORY omp LIBRARY_OUTPUT_DIRECTORY)
 endif()
 if(NOT LIBOMP_OUTPUT_DIRECTORY)
   set(LIBOMP_OUTPUT_DIRECTORY ${CMAKE_CURRENT_BINARY_DIR})

openmp/runtime/src/CMakeLists.txt

else()
     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}
   )
 
+  if(MSVC)
+    set(LIBOMP_IMP_LIB_TARGET ompimp)
+  else()
+    set(LIBOMP_IMP_LIB_TARGET omp)
+  endif()
+
   # Create def files to designate exported functions
   libomp_get_gdflags(LIBOMP_GDFLAGS) # generate-def.pl flags (Windows only)
   libomp_string_to_list("${LIBOMP_GDFLAGS}" LIBOMP_GDFLAGS)
   set(LIBOMP_GENERATED_DEF_FILE ${LIBOMP_LIB_NAME}.def)
   set(LIBOMPIMP_GENERATED_DEF_FILE_IMP ${LIBOMP_LIB_NAME}.imp.def)
   add_custom_command(
     OUTPUT  ${LIBOMP_LIB_NAME}.def
     # one with ordinals to use for building the runtime dll to maintain backwads compatible exports order
     COMMAND ${PERL_EXECUTABLE} ${LIBOMP_TOOLS_DIR}/generate-def.pl ${LIBOMP_GDFLAGS} -D NAME=${LIBOMP_LIB_FILE}
             -o ${LIBOMP_GENERATED_DEF_FILE} ${CMAKE_CURRENT_SOURCE_DIR}/dllexports
     DEPENDS ${CMAKE_CURRENT_SOURCE_DIR}/dllexports ${LIBOMP_TOOLS_DIR}/generate-def.pl
     # one without ordinals to use for building the import library to remove ordinal dependency going forward
     COMMAND ${PERL_EXECUTABLE} ${LIBOMP_TOOLS_DIR}/generate-def.pl ${LIBOMP_GDFLAGS} -D NAME=${LIBOMP_LIB_FILE} -D NOORDINALS
             -o ${LIBOMPIMP_GENERATED_DEF_FILE_IMP} ${CMAKE_CURRENT_SOURCE_DIR}/dllexports
     DEPENDS ${CMAKE_CURRENT_SOURCE_DIR}/dllexports ${LIBOMP_TOOLS_DIR}/generate-def.pl
   )
 
-  # Regenerate the import library to import by name, not ordinal
+  if (MSVC)
+    # Regenerate the import library to import by name, not ordinal.
+    #
+    # For mingw, we can't regenerate an import library by passing
+    # CMAKE_LINK_DEF_FILE_FLAG to the static library archiver; that just
+    # ends up creating a regular static library that contains the def file.
+    # For mingw, we would have to call the suitable dlltool for regenerating
+    # an import library. However, neither GNU nor LLVM based mingw tools
+    # generate import libraries that actually link by ordinal, so this step
+    # isn't strictly necessary.
+    #
+    # Also, in mingw builds, LIBOMP_GENERATED_IMP_LIB_FILENAME and
+    # LIBOMP_IMP_LIB_FILE currently end up equal, while they need to differ
+    # for this second step to work.
     add_library(ompimp STATIC ${LIBOMP_SOURCE_FILES})
       set_target_properties(ompimp PROPERTIES
         PREFIX "" SUFFIX "" OUTPUT_NAME "${LIBOMP_IMP_LIB_FILE}"
       LINKER_LANGUAGE C
     )
-  set_target_properties(ompimp PROPERTIES STATIC_LIBRARY_OPTIONS 
+    set_target_properties(ompimp PROPERTIES STATIC_LIBRARY_OPTIONS
         "${CMAKE_LINK_DEF_FILE_FLAG}${CMAKE_CURRENT_BINARY_DIR}/${LIBOMPIMP_GENERATED_DEF_FILE_IMP}"
     )
     add_dependencies(ompimp libomp-needed-headers)
-  
- 
+  endif()
 endif()
 
 # Building the Fortran module files
 # One compilation step creates both omp_lib.mod and omp_lib_kinds.mod
 if(${LIBOMP_FORTRAN_MODULES})
   configure_file(${LIBOMP_INC_DIR}/omp_lib.h.var omp_lib.h @ONLY)
   configure_file(${LIBOMP_INC_DIR}/omp_lib.f90.var omp_lib.f90 @ONLY)
   # Workaround for gfortran to build modules with the

 endif()
 add_dependencies(libomp-micro-tests libomp-test-deps)
 
 # Install rules
 # We want to install libomp in ${DESTDIR}/${CMAKE_INSTALL_FULL_LIBDIR}
 # We want to install headers in ${DESTDIR}/${CMAKE_INSTALL_FULL_INCLUDEDIR}
 if(WIN32)
   install(TARGETS omp RUNTIME DESTINATION "${CMAKE_INSTALL_BINDIR}")
-  install(TARGETS ompimp ARCHIVE DESTINATION "${OPENMP_INSTALL_LIBDIR}")
+  install(TARGETS ${LIBOMP_IMP_LIB_TARGET} ARCHIVE DESTINATION "${OPENMP_INSTALL_LIBDIR}")
   # Create aliases (regular copies) of the library for backwards compatibility
   set(LIBOMP_ALIASES "libiomp5md")
   foreach(alias IN LISTS LIBOMP_ALIASES)
     install(CODE "execute_process(COMMAND \"\${CMAKE_COMMAND}\" -E copy \"${LIBOMP_LIB_FILE}\"
       \"${alias}${LIBOMP_LIBRARY_SUFFIX}\" WORKING_DIRECTORY \"${CMAKE_INSTALL_FULL_BINDIR}\")")
     extend_path(outdir "${CMAKE_INSTALL_PREFIX}" "${OPENMP_INSTALL_LIBDIR}")
     install(CODE "execute_process(COMMAND \"\${CMAKE_COMMAND}\" -E copy \"${LIBOMP_IMP_LIB_FILE}\"
       \"${alias}${CMAKE_STATIC_LIBRARY_SUFFIX}\" WORKING_DIRECTORY \"${outdir}\")")