Skip to content

Fix CMake Bugs #57

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.

Sign up for GitHub

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

Closed
wants to merge 2 commits into from
Closed

Fix CMake Bugs #57

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
95f5e7e
Fix CMake Bugs
CascadingRadium Oct 27, 2025
afe2846
add gpu method and further fix gpu c_api
CascadingRadium Nov 4, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 3 additions & 1 deletion CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,9 @@ set(CMAKE_CXX_STANDARD 17)
list(APPEND CMAKE_MODULE_PATH "${PROJECT_SOURCE_DIR}/cmake")

# Valid values are "generic", "avx2", "avx512", "avx512_spr", "sve".
option(FAISS_OPT_LEVEL "" "generic")
if(NOT DEFINED FAISS_OPT_LEVEL)
set(FAISS_OPT_LEVEL "generic" CACHE STRING "Optimization level (generic, avx2, avx512, etc.)")
endif()
option(FAISS_ENABLE_GPU "Enable support for GPU indexes." ON)
option(FAISS_GPU_STATIC "Link GPU libraries statically." OFF)
option(FAISS_ENABLE_CUVS "Enable cuVS for GPU indexes." OFF)
Expand Down
24 changes: 10 additions & 14 deletions c_api/CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,13 +39,7 @@ set(FAISS_C_SRC
)

add_library(faiss_c ${FAISS_C_SRC})
if(FAISS_OPT_LEVEL STREQUAL "generic")
target_link_libraries(faiss_c PRIVATE faiss)
elseif(FAISS_OPT_LEVEL STREQUAL "avx2")
target_link_libraries(faiss_c PRIVATE faiss_avx2)
elseif(FAISS_OPT_LEVEL STREQUAL "avx512")
target_link_libraries(faiss_c PRIVATE faiss_avx512)
endif()
Copy link
Member

@abhinavdangeti abhinavdangeti Oct 27, 2025

Choose a reason for hiding this comment

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

And why are we getting rid of this - in our current builds we do produce libfaiss with the avx2 suffix when FAISS_OPT_LEVEL is set to avx2.

Copy link
Member Author

@CascadingRadium CascadingRadium Oct 27, 2025

Choose a reason for hiding this comment

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

basically earlier we had

  • faiss_c.so -> faiss.so, faiss_avx2.so, faiss_avx512.so etc.

in the latest upstream

  • faiss_c_avx2.so -> faiss_avx2.so
  • faiss_c.so -> faiss.so
  • faiss_c_avx512.so -> faiss_avx512.so

basically there are specialized _c libraries for each corresponding
faiss library.

Copy link
Member Author

@CascadingRadium CascadingRadium Oct 27, 2025

Choose a reason for hiding this comment

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

faiss/c_api/CMakeLists.txt

Line 45 in 95f5e7e

target_link_libraries(faiss_c_avx2 PRIVATE faiss_avx2)

Copy link
Member

@abhinavdangeti abhinavdangeti Oct 27, 2025

Choose a reason for hiding this comment

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

Right, and this is the bit I'm concerned with, it seems we setup faiss_c and faiss_avx2 currently ..

root@se2207-ubu20:~# ls -lart /opt/couchbase/lib/libfaiss_*
-rw-r--r-- 1 couchbase couchbase   204064 Mar 30  2010 /opt/couchbase/lib/libfaiss_c.so
-rw-r--r-- 1 couchbase couchbase 37985928 Mar 30  2010 /opt/couchbase/lib/libfaiss_avx2.so
root@se2207-ubu20:~# ldd /opt/couchbase/lib/libfaiss_c.so
	linux-vdso.so.1 (0x00007ffd65dd8000)
	libfaiss_avx2.so => /opt/couchbase/lib/libfaiss_avx2.so (0x00007f0bfb6ed000)
	libstdc++.so.6 => /opt/couchbase/lib/libstdc++.so.6 (0x00007f0bfb48d000)
	libm.so.6 => /lib/x86_64-linux-gnu/libm.so.6 (0x00007f0bfb334000)
	libgcc_s.so.1 => /opt/couchbase/lib/libgcc_s.so.1 (0x00007f0bfb30f000)
	libc.so.6 => /lib/x86_64-linux-gnu/libc.so.6 (0x00007f0bfb11d000)
	/lib64/ld-linux-x86-64.so.2 (0x00007f0bfdb6b000)
	libpthread.so.0 => /lib/x86_64-linux-gnu/libpthread.so.0 (0x00007f0bfb0fa000)
	libdl.so.2 => /lib/x86_64-linux-gnu/libdl.so.2 (0x00007f0bfb0f2000)
	libgomp.so.1 => /opt/couchbase/lib/libgomp.so.1 (0x00007f0bfb0a6000)

Will we not require to change anything on our build side on account of this?

target_link_libraries(faiss_c PRIVATE faiss)

add_library(faiss_c_avx2 ${FAISS_C_SRC})
target_link_libraries(faiss_c_avx2 PRIVATE faiss_avx2)
Expand Down Expand Up @@ -131,13 +125,15 @@ file(GLOB FAISS_C_API_HEADERS

faiss_install_headers("${FAISS_C_API_HEADERS}" c_api)

install(TARGETS faiss_c
EXPORT faiss-targets
RUNTIME DESTINATION ${CMAKE_INSTALL_BINDIR}
ARCHIVE DESTINATION ${CMAKE_INSTALL_LIBDIR}
LIBRARY DESTINATION ${CMAKE_INSTALL_LIBDIR}
INCLUDES DESTINATION ${CMAKE_INSTALL_INCLUDEDIR}
)
if(FAISS_OPT_LEVEL STREQUAL "generic")
Copy link
Member

@abhinavdangeti abhinavdangeti Oct 27, 2025

Choose a reason for hiding this comment

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

Not sure I follow this, don't we need to do call RUNTIME DESTINATION and INCLUDES DESTINATION for all FAISS_OPT_LEVELs?

Copy link
Member Author

@CascadingRadium CascadingRadium Oct 27, 2025

Choose a reason for hiding this comment

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

faiss/faiss/CMakeLists.txt

Line 428 in 95f5e7e

if(FAISS_OPT_LEVEL STREQUAL "generic")

this check for "generic" is there for the "faiss" sub project as well.
Mainly, if we use "avx2", we unnecessarily install the regular faiss_c as well.

install(TARGETS faiss_c
EXPORT faiss-targets
RUNTIME DESTINATION ${CMAKE_INSTALL_BINDIR}
ARCHIVE DESTINATION ${CMAKE_INSTALL_LIBDIR}
LIBRARY DESTINATION ${CMAKE_INSTALL_LIBDIR}
INCLUDES DESTINATION ${CMAKE_INSTALL_INCLUDEDIR}
)
endif()
if(FAISS_OPT_LEVEL STREQUAL "avx2")
install(TARGETS faiss_c_avx2
EXPORT faiss-targets
Expand Down