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

Problem with AVX MSVC flag #1114

Open
SimonRit opened this issue Jul 22, 2019 · 9 comments

Comments

@SimonRit
Copy link

commented Jul 22, 2019

Since commit 41203a5 and 9c6053e (of @rmukh and @hjmjohnson), RTK is not compiling on MSVC. See July 3 dashboard page, the commit appears in ITK after InITK-Windows7-64bit-MSVC-ITKmaster-Shared-RelWithDebInfo-FFTWON successfully compiled and before InITK-Windows7-64bit-MSVC-ITKmaster-Static-RelWithDebInfo-FFTWON-TBB fails (both succeed/fail the days before/after). I have no idea how to deal with this but if I remove these lines, it compiles fine, see this dashboard result... Is my machine too old for ITK (~2010) or is there an error in these cmake changes? Thanks for your help and let me know if I can test anything.

@SimonRit SimonRit added the type:Bug label Jul 22, 2019

@dzenanz

This comment has been minimized.

Copy link
Member

commented Jul 22, 2019

Since the error messages are mostly in French, I don't understand what they are saying. Maybe @jcfr could read them? But your machine might be too old to have these instructions. Does that happen on one machine only, or on multiple machines?

@SimonRit

This comment has been minimized.

Copy link
Author

commented Jul 22, 2019

Sorry about that but the messages are not informative. Two types for problems:

  • the files generated by gengetopt are not generated because gengetopt crashes. Gengetopt is compiled in the same build so the new instructions make it crash.
  • the tests crash.
    I don't have another Windows machine but the problems are not there on AppVeyor
@rmukh

This comment has been minimized.

Copy link
Contributor

commented Jul 23, 2019

@SimonRit It might be a problem with VS compiler, but not sure. You are using Visual Studio 14 2015 Win64, is it possible for you to test the same build, but with Visual Studio 15 2017 Win64? Thank you in advance.

@SimonRit

This comment has been minimized.

Copy link
Author

commented Jul 24, 2019

Yes, and the same thing seems to occur. See Experimental build here.

@rmukh

This comment has been minimized.

Copy link
Contributor

commented Jul 24, 2019

Thank you for sharing results.

Then, you are probably right that 'the files generated by gengetopt are not generated because gengetopt crashes. Gengetopt is compiled in the same build so the new instructions make it crash.'

I will take a look at possible solutions for that particular build configuration in free time.

@hjmjohnson

This comment has been minimized.

Copy link
Member

commented Jul 24, 2019

You should be able to set the cache values for the compiler optimizations to something suitable for this old machine. As a quick solution, simply setting them to empty values should be enough to tell the compiler to avoid using instructions that are too new for your computer.

-DITK_C_OPTIMIZATION_FLAGS:STRING=""
-DITK_CXX_OPTIMIZATION_FLAGS:STRING=""
@SimonRit

This comment has been minimized.

Copy link
Author

commented Jul 25, 2019

A quick update. I have added the following code to my ctest script

file(READ "${CTEST_SOURCE_DIRECTORY}\\CMake\\ITKSetStandardCompilerFlags.cmake" ITKSETSTANDARDCOMPILERFLAGS_FILE)
string(REGEX REPLACE "/arch:AVX " "\"\"" ITKSETSTANDARDCOMPILERFLAGS_FILE "${ITKSETSTANDARDCOMPILERFLAGS_FILE}")
string(REGEX REPLACE "/arch:AVX2" "\"\"" ITKSETSTANDARDCOMPILERFLAGS_FILE "${ITKSETSTANDARDCOMPILERFLAGS_FILE}")
file(WRITE "${CTEST_SOURCE_DIRECTORY}\\CMake\\ITKSetStandardCompilerFlags.cmake" "${ITKSETSTANDARDCOMPILERFLAGS_FILE}")

and the compilation worked fine last night. This computer is still less than 10 years so I would suggest to detect the availability of the avx instructions... Maybe this piece of code can help:
https://gist.github.com/UnaNancyOwen/263c243ae1e05a2f9d0e

What do you think?

@dzenanz

This comment has been minimized.

Copy link
Member

commented Jul 25, 2019

While we might do that, it adds more configure-time tests, when initial configuration time on Windows is on the order of 5 minutes or so. And it also needs to be maintained. @hjmjohnson @rmukh what do you think?

@hjmjohnson

This comment has been minimized.

Copy link
Member

commented Jul 25, 2019

I have mixed feelings on this, but I think that in this case the added complexity is probably worth the extra few seconds of testing.

I don't have time in the near future to address this :(.

I think the quick solution might be something like

https://cmake.org/cmake/help/v3.10/command/cmake_host_system_information.html

cmake_host_system_information(RESULT MSVC_SUPPORTS_AVX QUERY HAS_SSE)
cmake_host_system_information(RESULT MSVC_SUPPORTS_AVX2 QUERY HAS_SSE2)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants
You can’t perform that action at this time.