ABI issues + link error on MinGW target

[nu774]

Hi, found several issues on MinGW target. Patch is attached.

1. Build with OpenMP results in link time error, since gcc requires -fopenmp on link time but it is not set. This can be fixed by setting target LINK_FLAGS property to $OpenMP_C_FLAGS, but I don't know if it's appropriate for other compilers/linkers than GNU C. It seems at least OK for MSVC (Although MSVC linker doesn't need -openmp switch, it's just gracefully ignored with a warning).
2. Found several ABI weakness on MinGW DLL. At first, GNUC *assumes* 16byte stack frame alignment by dafault, but it's not assured by x86 ABI, and MSVC doesn't align stack frames as such. As a result, calling SIMD enabled function in MinGW DLL from MSVC program results in segfaults, due to stack frame misalignment. This can be avoided by setting -mincoming-stack-boundary=2.
3. Secondly, calling convention for function returning structure of Mingw-GCC changed at version 4.7, and it breaks compatibility. Read https://sourceforge.net/projects/mingw/files/MinGW/Base/gcc/Version4/gcc-4.7.2-1/ for details. Since fixing this breaks API, I don't think fixing this is a MUST.

The following is the patch I tried.

diff --git a/src/CMakeLists.txt b/src/CMakeLists.txt
index 2a21156..cc83a27 100644
--- a/src/CMakeLists.txt
+++ b/src/CMakeLists.txt
@@ -28,6 +27,9 @@ endif ()

 if (HAVE_SIMD)
   set (SIMD_SOURCES rate32s ${RDFT32S} simd)
+  if (CMAKE_COMPILER_IS_GNUCC)
+    set (SIMD_C_FLAGS "${SIMD_C_FLAGS} -mincoming-stack-boundary=2")
+  endif ()
   foreach (source ${SIMD_SOURCES})
     set_property (SOURCE ${source} PROPERTY COMPILE_FLAGS ${SIMD_C_FLAGS})
   endforeach ()
@@ -45,6 +47,18 @@ set_target_properties (${PROJECT_NAME} PROPERTIES
   INSTALL_NAME_DIR ${LIB_INSTALL_DIR}
   LINK_INTERFACE_LIBRARIES ""
   PUBLIC_HEADER "${PROJECT_NAME}.h")
+
+if (WITH_OPENMP)
+    get_target_property(_existing ${PROJECT_NAME} LINK_FLAGS)
+    if (_existing)
+        set (TARGET_LD_FLAGS "${_existing} ${OpenMP_C_FLAGS}")
+    else ()
+        set (TARGET_LD_FLAGS "${OpenMP_C_FLAGS}")
+    endif ()
+    set_target_properties(${PROJECT_NAME} PROPERTIES
+        LINK_FLAGS "${TARGET_LD_FLAGS}")
+endif()
+
 if (BUILD_FRAMEWORK)
   set_target_properties (${PROJECT_NAME} PROPERTIES FRAMEWORK TRUE)
 elseif (NOT WIN32)

[nu774]

Hmm, setting -mincomming-stack-boundary=2 on x86_64 target seems to result in compile error.
More finer condition is required by investigating target arch (we only need it for 32bit x86 target).

[nu774]

Updated patch.

diff --git a/src/CMakeLists.txt b/src/CMakeLists.txt
index 2a21156..ff40e1b 100644
--- a/src/CMakeLists.txt
+++ b/src/CMakeLists.txt
@@ -28,6 +27,13 @@ endif ()

 if (HAVE_SIMD)
   set (SIMD_SOURCES rate32s ${RDFT32S} simd)
+  if (CMAKE_COMPILER_IS_GNUCC)
+    include (CheckCCompilerFlag)
+    CHECK_C_COMPILER_FLAG("-mincoming-stack-boundary=2" HAVE_STACK_BOUNDARY_2)
+    if (HAVE_STACK_BOUNDARY_2)
+      set (SIMD_C_FLAGS "${SIMD_C_FLAGS} -mincoming-stack-boundary=2")
+    endif ()
+  endif ()
   foreach (source ${SIMD_SOURCES})
     set_property (SOURCE ${source} PROPERTY COMPILE_FLAGS ${SIMD_C_FLAGS})
   endforeach ()
@@ -45,6 +51,18 @@ set_target_properties (${PROJECT_NAME} PROPERTIES
   INSTALL_NAME_DIR ${LIB_INSTALL_DIR}
   LINK_INTERFACE_LIBRARIES ""
   PUBLIC_HEADER "${PROJECT_NAME}.h")
+
+if (WITH_OPENMP)
+    get_target_property(_existing ${PROJECT_NAME} LINK_FLAGS)
+    if (_existing)
+        set (TARGET_LD_FLAGS "${_existing} ${OpenMP_C_FLAGS}")
+    else ()
+        set (TARGET_LD_FLAGS "${OpenMP_C_FLAGS}")
+    endif ()
+    set_target_properties(${PROJECT_NAME} PROPERTIES
+        LINK_FLAGS "${TARGET_LD_FLAGS}")
+endif()
+
 if (BUILD_FRAMEWORK)
   set_target_properties (${PROJECT_NAME} PROPERTIES FRAMEWORK TRUE)
 elseif (NOT WIN32)

[robs]

Hello and thanks for these suggestions. For the first issue, please can you let me know if the following patch works for you?

diff --git a/cmake/Modules/FindSIMD.cmake b/cmake/Modules/FindSIMD.cmake
index 6484bbd..6ac51cb 100644
--- a/cmake/Modules/FindSIMD.cmake
+++ b/cmake/Modules/FindSIMD.cmake
@@ -40,12 +40,18 @@
include (CheckCSourceCompiles)
include (FindPackageHandleStandardArgs)

+if (WIN32) # Safety for when mixed lib/app compilers (but performance hit)
+ set (GCC_WIN32_SIMD_OPTS "-mincoming-stack-boundary=2")
+endif ()
+
set (SIMD_C_FLAG_CANDIDATES
- # Microsoft Visual Studio x64
+ # x64
" "
# Microsoft Visual Studio x86
"/arch:SSE /fp:fast -D__SSE__"
- # Gnu
+ # Gcc x86
+ "-msse -mfpmath=sse ${GCC_WIN32_SIMD_OPTS}"
+ # Gcc x86 (old versions)
"-msse -mfpmath=sse"
)

[robs]

and maybe this for the second issue:

diff --git a/CMakeLists.txt b/CMakeLists.txt
index 8a24952..7bfcd97 100644
--- a/CMakeLists.txt
+++ b/CMakeLists.txt
@@ -79,6 +79,9 @@ endif ()
if (OPENMP_FOUND)
set (CMAKE_C_FLAGS "${CMAKE_C_FLAGS} ${OpenMP_C_FLAGS}")
set (CMAKE_EXE_LINKER_FLAGS "${CMAKE_EXE_LINKER_FLAGS} ${OpenMP_EXE_LINKER_FLAGS}")
+ if (MINGW)
+ set (CMAKE_SHARED_LINKER_FLAGS "${CMAKE_SHARED_LINKER_FLAGS} ${OpenMP_C_FLAGS}")
+ endif ()
endif ()

if (WITH_SIMD)

[nu774]

Thanks, it works fine here.
And sorry for patch formatting. Since all whitespaces at line beginning are stripped off, copy & paste doesn't seem to work.

One more thing. On i686-w64-mingw target, __divdi3() is imported from libgcc.
Requiring extra libgcc DLL for merely one function looks somewhat wasteful.
Is it possible to set -static-libgcc for MinGW target?