Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Win32/64: error: inlining failed in call with (and without) -march=native #466

Closed
anonimal opened this issue Aug 24, 2017 · 30 comments
Closed

Comments

@anonimal
Copy link
Contributor

When building b3cacd8 with -march=native on Win32/64:

@anonimal anonimal changed the title Win32/64: error: inlining failed in call to always_inline '__m128i _mm_sha1nexte_epu32(__m128i, __m128i)': target specific option mismatch Win32/64: error: inlining failed in call with -march=native Aug 24, 2017
@anonimal anonimal changed the title Win32/64: error: inlining failed in call with -march=native Win32/64: error: inlining failed in call with (and without) -march=native Aug 24, 2017
@anonimal
Copy link
Contributor Author

Same machines without -march=native:

Win32: gcc 6.2.0
Win64: gcc 7.2.0

@noloader
Copy link
Collaborator

noloader commented Aug 24, 2017

@anonimal,

I don't have a test rig set-up for this configuration. I suspect MinGW is mis-detecting around CMakeLists.txt : 100, and then failing to enter this block around CMakeLists.txt : 370.

What does g++ -dumpmachine return on MinGW? Does this change look about right: Commit babcc1f0b705?

@anonimal
Copy link
Contributor Author

What does g++ -dumpmachine return on MinGW?

  • i686-w64-mingw32
  • x86_64-w64-mingw32

Does this change look about right: Commit babcc1f?

Building against this revision produces the same results.

@noloader
Copy link
Collaborator

noloader commented Aug 24, 2017

Thanks @anonimal,

So I tried to simplify things a bit at Commit 48ec6946d729.

If that fails, would you be able to add a few message(...) around line 400 of CMakeList.txt. I'd like to ensure we are entering the two if blocks:

# New as of Pull Request 461, http://github.com/weidai11/cryptopp/pull/461.
if (("${CMAKE_CXX_COMPILER_ID}" STREQUAL "Clang") OR ("${CMAKE_CXX_COMPILER_ID}" STREQUAL "GNU"))

   message(STATUS "Clang or GNU")

   if (("${CRYPTOPP_AMD64}" STREQUAL "1") OR ("${CRYPTOPP_I386}" STREQUAL "1") OR ("${CRYPTOPP_X32}" STREQUAL "1"))

       message(STATUS "Intel IA32")
       ...
    endif()
endif()

@noloader
Copy link
Collaborator

noloader commented Aug 24, 2017

Cleared at PR 469. (We can re-open if required).

@anonimal
Copy link
Contributor Author

I'd like to ensure we are entering the two if blocks:

Apparently, it's not entering the 2nd block (see logs below).

noloader@48ec694

Unresolved inline errors:

@noloader
Copy link
Collaborator

noloader commented Aug 24, 2017

Apparently, it's not entering the 2nd block (see logs below).

According to the logs, its not entering the first block either. Or at least the Win64 machine is not.

Can you work this out and submit a PR? I bet CMAKE_CXX_COMPILER_ID is not set to what we expect.

If you don't have the time to investigate, then try this:

message("CMAKE_CXX_COMPILER_ID: ${CMAKE_CXX_COMPILER_ID}")
if (("${CMAKE_CXX_COMPILER_ID}" STREQUAL "Clang") OR ("${CMAKE_CXX_COMPILER_ID}" STREQUAL "GNU"))
    ...
endif()

I found a Stack Overflow question similar to this problems at CMAKE_COMPILER_IS_GNUCXX and CMAKE_CXX_COMPILER_ID are empty, but it felt like it did not apply.

@noloader noloader reopened this Aug 24, 2017
@anonimal
Copy link
Contributor Author

According to the logs, its not entering the first block either. Or at least the Win64 machine is not.

Damnit, sorry, I uploaded the wrong log. That log is pre-patch. Will re-up.

If you don't have the time to investigate, then try this:

Will do.

@anonimal
Copy link
Contributor Author

The log I wanted to upload in #466 (comment) + patch in #466 (comment):

@noloader
Copy link
Collaborator

noloader commented Aug 24, 2017

The log I wanted to upload in #466 (comment) + patch in #466 (comment):

OK, thanks. Does that mean i686 is OK, but x86_64 is a problem?

If so, could you look at it and tell me what we need?

@anonimal
Copy link
Contributor Author

OK, thanks. Does that mean i686 is OK, but x86_64 is a problem?

No, #466 (comment) was simply an addendum to #466 (comment). Both issues remain for i686 and x86_64.

@noloader
Copy link
Collaborator

noloader commented Aug 24, 2017

No, #466 (comment) was simply an addendum to #466 (comment). Both issues remain for i686 and x86_64.

Sorry about that. I don't have a test rig so there's not much more I can do. Maybe someone more experienced with CMake and access to MinGW can help out.

@anonimal
Copy link
Contributor Author

@noloader I'll see if I can get you access to our win machines (if at least temporarily). It will be quicker than me looking at this issue closer. Worst case scenario being I'll set time aside to dive into a fix.

@noloader
Copy link
Collaborator

I'll see if I can get you access to our win machines...

Yeah, that would be fine. Here's my authorized_keys file.

Something else that may work... Build out a Virtual Box image with MinGW installed and configured to your taste. I'm currently running Virtual Box version 5.1.26 r117224 (Qt5.6.2). Its the latest.

@anonimal
Copy link
Contributor Author

JFTR: message(STATUS ${CRYPTOPP_MINGW32}) (and the W64 equivalent), after being set by OUTPUT_VARIABLE, produce blank lines (neither 1 nor 0 as expected).

@noloader
Copy link
Collaborator

@anonimal,

message(STATUS ${CRYPTOPP_MINGW32}) (and the W64 equivalent), after being set by OUTPUT_VARIABLE, produce blank lines (neither 1 nor 0 as expected).

Well, that does not make much sense. Does MinGW have egrep available? Maybe run it by hand:

c++ -dumpmachine | egrep -i -c w64-mingw32

If it looks right, then maybe OUTPUT_STRIP_TRAILING_WHITESPACE is doing something funky?

@anonimal
Copy link
Contributor Author

Well, that does not make much sense. Does MinGW have egrep available? Maybe run it by hand:

Yes, and running the above produces expected results (I've gotten used to being disappointed by CMake by now 😒).

If it looks right, then maybe OUTPUT_STRIP_TRAILING_WHITESPACE is doing something funky?

I've removed OUTPUT_STRIP_TRAILING_WHITESPACE to no avail (same results).

@noloader
Copy link
Collaborator

noloader commented Aug 25, 2017

I opened a question on Stack Overflow at Variable is empty after executing command on MinGW and MinGW-64?.

Would you happen to know what versions of CMake are being used on MinGW and MinGW-64? Can you ask your customer for it?

@noloader
Copy link
Collaborator

noloader commented Aug 25, 2017

A fellow named VRE on Stack Overflow offered an answer at Variable is empty after executing command on MinGW and MinGW-64? I committed VRE's answer on my testing branch at Commit 750553570eaa. Can you give it a try?

@noloader
Copy link
Collaborator

@anonimal,

I committed the fix to Wei's repo at Commit 77e9d8c2dbd9. It tested OK under Cygwin, Linux, OS X, Solaris and Windows. At worse, it won't do any harm.

I'm going to close this out. Ping me if its still a problem. We can reopen the report again :)

noloader referenced this issue Aug 25, 2017
We don't know if this is going to fix the issue because we don't have a MinGW platofrm for testing. However, from VRE's answer on Stack Overflow (and the chronic CMake problems with execute_process), we believe this may be the fix.
The fix tested OK on WIndows, Linux, OS X and Cygwin. At worse, it won't do any harm
noloader referenced this issue Aug 25, 2017
Rollup CMake fixes for OpenBSD (Issues 467-468)
Rollup CMake fixes for MinGW (Issues 466)
Add CRYPTOPP_ENABLE_ARCH (Issue 380)
@anonimal
Copy link
Contributor Author

The problem persists 😡

@anonimal
Copy link
Contributor Author

To resolve any syntactical doubt:

diff --git a/CMakeLists.txt b/CMakeLists.txt
index c5c10bf..28d1527 100644
--- a/CMakeLists.txt
+++ b/CMakeLists.txt
@@ -113,11 +113,19 @@ execute_process(COMMAND ${SHELL_CMD} ${SHELL_ARGS} "${CMAKE_CXX_COMPILER} -dumpm
        OUTPUT_VARIABLE CRYPTOPP_AMD64
        OUTPUT_STRIP_TRAILING_WHITESPACE)
 
+      message(STATUS CRYPTOPP_AMD64)^M
+      message(STATUS ${CRYPTOPP_AMD64})^M
+      message(STATUS "${CRYPTOPP_AMD64}")^M
+^M
 execute_process(COMMAND ${SHELL_CMD} ${SHELL_ARGS} "${CMAKE_CXX_COMPILER} -dumpmachine 2>&1"
        COMMAND ${GREP_CMD} ${GREP_ARGS} "x86_64"
        OUTPUT_VARIABLE CRYPTOPP_X86_64
        OUTPUT_STRIP_TRAILING_WHITESPACE)
 
+      message(STATUS CRYPTOPP_X86_64)^M
+      message(STATUS ${CRYPTOPP_X86_64})^M
+      message(STATUS "${CRYPTOPP_X86_64}")^M
+^M
 execute_process(COMMAND ${SHELL_CMD} ${SHELL_ARGS} "${CMAKE_CXX_COMPILER} -dumpmachine 2>&1"
        COMMAND ${GREP_CMD} ${GREP_ARGS} "i.86"
        OUTPUT_VARIABLE CRYPTOPP_I386
@@ -135,6 +143,10 @@ execute_process(COMMAND ${SHELL_CMD} ${SHELL_ARGS} "${CMAKE_CXX_COMPILER} -dumpm
        OUTPUT_VARIABLE CRYPTOPP_MINGW64
        OUTPUT_STRIP_TRAILING_WHITESPACE)
 
+      message(STATUS CRYPTOPP_MINGW64)^M
+      message(STATUS ${CRYPTOPP_MINGW64})^M
+      message(STATUS "${CRYPTOPP_MINGW64}")^M
+^M
 execute_process(COMMAND ${SHELL_CMD} ${SHELL_ARGS} "${CMAKE_CXX_COMPILER} -dumpmachine 2>&1"
        COMMAND ${GREP_CMD} ${GREP_ARGS} "x32"
        OUTPUT_VARIABLE CRYPTOPP_X32
@@ -405,8 +417,12 @@ endif()
 # New as of Pull Request 461, http://github.com/weidai11/cryptopp/pull/461.
 if (("${CMAKE_CXX_COMPILER_ID}" STREQUAL "Clang") OR ("${CMAKE_CXX_COMPILER_ID}" STREQUAL "GNU"))
 
+  message("first block")^M
+^M
        if (("${CRYPTOPP_AMD64}" STREQUAL "1") OR ("${CRYPTOPP_I386}" STREQUAL "1") OR ("${CRYPTOPP_X32}" STREQUAL "1"))
 
+          message("second block")^M
+^M
                CHECK_CXX_COMPILER_FLAG("-mssse3" CRYPTOPP_IA32_SSSE3)
                CHECK_CXX_COMPILER_FLAG("-msse4.2" CRYPTOPP_IA32_SSE4)
                CHECK_CXX_COMPILER_FLAG("-mssse3 -mpclmul" CRYPTOPP_IA32_CLMUL)

On a sane Linux system with CMake 3.9.1:

-- The C compiler identification is GNU 7.1.1                                                                                                                                                                [3/7]
-- The CXX compiler identification is GNU 7.1.1                                                                                                                                                                   
-- Check for working C compiler: /usr/bin/cc                                                                                                                                                                      
-- Check for working C compiler: /usr/bin/cc -- works                                                                                                                                                             
-- Detecting C compiler ABI info                    
-- Detecting C compiler ABI info - done             
-- Detecting C compile features                     
-- Detecting C compile features - done              
-- Check for working CXX compiler: /usr/bin/c++     
-- Check for working CXX compiler: /usr/bin/c++ -- works                                                 
-- Detecting CXX compiler ABI info                  
-- Detecting CXX compiler ABI info - done           
-- Detecting CXX compile features                   
-- Detecting CXX compile features - done            
-- CRYPTOPP_AMD64                                   
-- 0                                                
-- 0                                                
-- CRYPTOPP_X86_64                                  
-- 1                                                
-- 1                                                
-- CRYPTOPP_MINGW64                                 
-- 0                                                
-- 0                                                
first block                                         
second block                                        
-- Performing Test CRYPTOPP_IA32_SSSE3              
-- Performing Test CRYPTOPP_IA32_SSSE3 - Success    
-- Performing Test CRYPTOPP_IA32_SSE4               
-- Performing Test CRYPTOPP_IA32_SSE4 - Success     
-- Performing Test CRYPTOPP_IA32_CLMUL              
-- Performing Test CRYPTOPP_IA32_CLMUL - Success    
-- Performing Test CRYPTOPP_IA32_AES                
-- Performing Test CRYPTOPP_IA32_AES - Success      
-- Performing Test CRYPTOPP_IA32_SHA                
-- Performing Test CRYPTOPP_IA32_SHA - Success      
-- Performing Test CRYPTOPP_IA32_NATIVE             
-- Performing Test CRYPTOPP_IA32_NATIVE - Success   
-- Looking for pthread.h                            
-- Looking for pthread.h - found                    
-- Looking for pthread_create                       
-- Looking for pthread_create - not found           
-- Looking for pthread_create in pthreads           
-- Looking for pthread_create in pthreads - not found                                                    
-- Looking for pthread_create in pthread            
-- Looking for pthread_create in pthread - found    
-- Found Threads: TRUE                              
-- Compiler:                                        
-- Flags:                                           
-- Build type: RelWithDebInfo                       
-- The following OPTIONAL packages have been found: 

 * Threads

-- Configuring done
-- Generating done

On MinGW-w64 with CMake 3.9.1:

-- The C compiler identification is GNU 7.2.0       
-- The CXX compiler identification is GNU 7.2.0     
-- Check for working C compiler: C:/msys64/mingw64/bin/gcc.exe                                           
-- Check for working C compiler: C:/msys64/mingw64/bin/gcc.exe -- works                                  
-- Detecting C compiler ABI info                    
-- Detecting C compiler ABI info - done             
-- Detecting C compile features                     
-- Detecting C compile features - done              
-- Check for working CXX compiler: C:/msys64/mingw64/bin/g++.exe                                         
-- Check for working CXX compiler: C:/msys64/mingw64/bin/g++.exe -- works                                
-- Detecting CXX compiler ABI info                  
-- Detecting CXX compiler ABI info - done           
-- Detecting CXX compile features                   
-- Detecting CXX compile features - done            
-- CRYPTOPP_AMD64                                   
--                                                  
--                                                  
-- CRYPTOPP_X86_64                                  
--                                                  
--                                                  
-- CRYPTOPP_MINGW64                                 
--                                                  
--                                                  
first block                                         
-- Looking for pthread.h                            
-- Looking for pthread.h - found                    
-- Looking for pthread_create                       
-- Looking for pthread_create - found               
-- Found Threads: TRUE                              
-- Compiler:                                        
-- Flags:                                           
-- Build type: RelWithDebInfo                       
-- The following OPTIONAL packages have been found: 

 * Threads                                          

-- Configuring done                                 
-- Generating done

I think we can resolve this as a CMake issue?

@anonimal
Copy link
Contributor Author

Could we reopen this issue until the problem is resolved upstream or until we refactor a way around this CMake issue?

@noloader noloader reopened this Aug 25, 2017
@noloader
Copy link
Collaborator

noloader commented Aug 25, 2017

@anonimal,

Could we reopen this issue until the problem is resolved upstream or until we refactor a way around this CMake issue?

Yes, absolutely. I thought we had it licked with the Stack Overflow answer. Sorry about the [second] premature close.

I think we can resolve this as a CMake issue?

Yeah, maybe. But we need to fix it since we supply the CMakefile.txt file.

One thing you might try is, quote every argument. The documentation on execute_process does not state its needed, but Windows' CreateProcess may want it. The docs also fail to state if a comma is needed to separate arguments.

I asked the CMake folks to update their documentation on their mailing list at Please update the documentation for execute_process.

I'll put a bounty on the question when Stack Overflow allows it. I want to get this CMake shit off the radar.

@noloader
Copy link
Collaborator

noloader commented Aug 26, 2017

@anonimal ,

I've gotten used to being disappointed by CMake by now ...

Lol... My blood pressure boils when I think about it. I can feel the veins in my neck throbbing when I think of the problems its causing our users.

The program is responsible for 17.5% of our bugs (27 of 154). That's more effort than we expend on things like compatibility back to GCC 3 and Visual Studio 2002. 17.5% is why we removed it from our release tar/zip. It introduces too many unknowns into our equations.

@noloader
Copy link
Collaborator

noloader commented Aug 26, 2017

@anonimal,

The problem persists...

How about unrolling everything for CMake under MinGW? Maybe something like:

execute_process(COMMAND sh -c "${CMAKE_CXX_COMPILER} -dumpmachine 2>&1"
        COMMAND grep -i -c "mingw32"
        OUTPUT_VARIABLE CRYPTOPP_MINGW32
        OUTPUT_STRIP_TRAILING_WHITESPACE)

Also, what if the tabs are causing it problems under MinGW? Should we try spaces?

What if the shell is wrong? Should we use MinGW's equivalent to cmd.exe or sh? Do you even know what its called? Is it term.exe?


Also, check this out under Cygwin (not MinGW):

$ c++ -dumpmachine
i686-pc-cygwin

$ sh -c c++ -dumpmachine
c++: fatal error: no input files
compilation terminated.

$ sh -c "c++ -dumpmachine"
i686-pc-cygwin

Maybe we need to quote the whole string argument given to sh -c?

Another open question is, can we drop the sh -c altogether? The second command in the CMake statement does not use it, so why doe the first need it?

@anonimal
Copy link
Contributor Author

How about unrolling everything for CMake under MinGW? Maybe something like:
Also, what if the tabs are causing it problems under MinGW? Should we try spaces?

:rage1: :rage2: :rage3: :rage4: this appears to solve the issue. I'll narrow down the specific cause and send a patch a.s.a.p (I didn't test each step successively, I simply clobbered together both possible solutions).

Thanks for your patience @noloader.

@anonimal
Copy link
Contributor Author

Lol... My blood pressure boils when I think about it. I can feel the veins in my neck throbbing when I think of the problems its causing our users.

I know that feeling! Especially with their "resolution" to https://gitlab.kitware.com/cmake/cmake/issues/16547

@noloader
Copy link
Collaborator

noloader commented Aug 26, 2017

@anonimal ,

Thanks for fixing the issue on MinGW and the PR.

I folded the machine tests and got rid of the if statements:

DumpMachine(CRYPTOPP_AMD64 "amd64|x86_64")

I also did away with ${SHELL_CMD} ${SHELL_ARGS}. It seems like another point of failure we don't need. Its hard coded now.

The changes tested OK in my test lab, including Solaris, OS X, ARM and Aarch64.

@noloader
Copy link
Collaborator

Last call for CMake patches before removing it from the library and adding it to the patch page. If you have something to add, then now is the time to do it.

Also see Remove CMake from GitHub; add it to Patch Page on wiki? and Commit 1c740b0a097aecaa.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants