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

cmake: misc fixes for build on i386 #15516

Merged
merged 1 commit into from Jun 7, 2017
Merged

Conversation

javacruft
Copy link
Contributor

Limit scope of SIMD detection to 64 bit x86 for:

HAVE_INTEL_PCLMUL
HAVE_INTEL_SSE4_1
HAVE_INTEL_SSE4_2

resolving build failures in gf-complete on i386; this is
inline with the previous autotools based feature detection.

Only compile crypto/isa-l if yasm 64 bit complier is found,
effectively limiting scope to x86_64.

@tchaikov tchaikov self-requested a review June 6, 2017 14:47
@@ -91,6 +91,7 @@ elseif(CMAKE_SYSTEM_PROCESSOR MATCHES "i386|i686|amd64|x86_64|AMD64")
if(HAVE_INTEL_SSSE3)
set(SIMD_COMPILE_FLAGS "${SIMD_COMPILE_FLAGS} -mssse3")
endif()
elseif(CMAKE_SYSTEM_PROCESSOR MATCHES "amd64|x86_64|AMD64")
Copy link
Contributor

@tchaikov tchaikov Jun 6, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i don't think this would work. as if the branches above are not mutual exclusive.

instead, probably we should just drop the i386 at line 76. because it apparently does not feature any of the SSE* instruction set. but we can have a dedicated branch for i686, as it supports SSE. see https://en.wikipedia.org/wiki/P6_(microarchitecture) and https://gcc.gnu.org/onlinedocs/gcc-4.5.3/gcc/i386-and-x86_002d64-Options.html.

and move SSE2 and above into the branch of amd64 x86_64 AMD64.

in otherwords:

if(CMAKE_SYSTEM_PROCESSOR MATCHES "i386|i686|amd64|x86_64|AMD64")
  set(HAVE_INTEL 1)
  if(CMAKE_SYSTEM_PROCESSOR MATCHES "i686|amd64|x86_64|AMD64")
    CHECK_C_COMPILER_FLAG(-msse HAVE_INTEL_SSE)
    if(HAVE_INTEL_SSE)
      set(SIMD_COMPILE_FLAGS "${SIMD_COMPILE_FLAGS} -msse")
    endif()
    if(CMAKE_SYSTEM_PROCESSOR MATCHES "amd64|x86_64|AMD64")
      if(HAVE_INTEL_SSE2)
        set(SIMD_COMPILE_FLAGS "${SIMD_COMPILE_FLAGS} -msse2")
      endif()
      CHECK_C_COMPILER_FLAG(-msse3 HAVE_INTEL_SSE3)
      if(HAVE_INTEL_SSE3)
        set(SIMD_COMPILE_FLAGS "${SIMD_COMPILE_FLAGS} -msse3")
      endif()
      CHECK_C_COMPILER_FLAG(-mssse3 HAVE_INTEL_SSSE3)
      if(HAVE_INTEL_SSSE3)
        set(SIMD_COMPILE_FLAGS "${SIMD_COMPILE_FLAGS} -mssse3")
      endif()
      # ...
    endif(CMAKE_SYSTEM_PROCESSOR MATCHES "amd64|x86_64|AMD64")
  endif(CMAKE_SYSTEM_PROCESSOR MATCHES "i686|amd64|x86_64|AMD64")
endif(CMAKE_SYSTEM_PROCESSOR MATCHES "i386|i686|amd64|x86_64|AMD64")

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You're quite correct - I'll rework

Scope SIMD CPU flag detection base on target architecture,
resolving build failures in gf-complete on i386; this is
inline with the previous autotools based feature detection.

Only compile crypto/isa-l if yasm 64 bit complier is found,
effectively limiting scope to x86_64.

Signed-off-by: James Page <james.page@ubuntu.com>
@tchaikov tchaikov changed the title build: misc fixes for cmake build on i386 cmake: misc fixes for build on i386 Jun 6, 2017
@liewegas liewegas merged commit c011314 into ceph:master Jun 7, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants