Skip to content

Conversation

@CascadingRadium
Copy link
Member

  • Fix default logic when no optimization level is set, set to "generic".
  • Fix c_api library linkage and installation in case of optimization levels.
    • faiss_c -> faiss in case of generic.
    • faiss_c_avx2 -> faiss_avx2 in case of AVX2.

LIBRARY DESTINATION ${CMAKE_INSTALL_LIBDIR}
INCLUDES DESTINATION ${CMAKE_INSTALL_INCLUDEDIR}
)
if(FAISS_OPT_LEVEL STREQUAL "generic")
Copy link
Member

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

Choose a reason for hiding this comment

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

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.

Copy link
Member

@abhinavdangeti abhinavdangeti left a comment

Choose a reason for hiding this comment

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

I think @ceejatec should weigh in on this too.

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

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

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

Choose a reason for hiding this comment

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

target_link_libraries(faiss_c_avx2 PRIVATE faiss_avx2)

Copy link
Member

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?

@abhinavdangeti
Copy link
Member

@CascadingRadium Per our conversation let's drop the changes from this PR that'll force us to use different ldflags downstream.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants