Skip to content

Commit 1f0fffb

Browse files
Michael Norrisfacebook-github-bot
authored andcommitted
Fix cuVS build (#5107)
Summary: D96034911 adds filtered search to BinaryCuvsCagra.cu (calling cuvs::neighbors::cagra::search with a filter_ref), but does not update fbcode/faiss/gpu/CMakeLists.txt. In the cmake build, cuVS source files are compiled with -fvisibility=hidden to ensure that RAFT/cuVS function calls resolve locally within libfaiss.so rather than through the PLT to libcuvs.so. This is critical because RAFT resources (cublas handles, CUDA streams) must be created and used within the same dynamic library context. Look at fbcode/faiss/gpu/CMakeLists.txt:302-316: set_source_files_properties( GpuIndexCagra.cu # ✓ float CAGRA GpuDistance.cu GpuIndexIVFFlat.cu GpuIndexIVFPQ.cu GpuIndexFlat.cu StandardGpuResources.cpp impl/CuvsCagra.cu # ✓ float CAGRA impl impl/CuvsFlatIndex.cu impl/CuvsIVFFlat.cu impl/CuvsIVFPQ.cu utils/CuvsFilterConvert.cu utils/CuvsUtils.cu TARGET_DIRECTORY faiss PROPERTIES COMPILE_OPTIONS "-fvisibility=hidden") Missing: - GpuIndexBinaryCagra.cu - impl/BinaryCuvsCagra.cu Before D96034911, this didn't matter because GpuIndexBinaryCagra::search() threw on any params (FAISS_THROW_IF_NOT_MSG(!params, "params not implemented")). After D96034911, it processes params and calls cuvs::neighbors::cagra::search with a filter, making the code sensitive to cross-DSO symbol resolution. Without hidden visibility, cuVS template instantiations (particularly cagra::search<uint8_t, uint32_t> with the filter type) leak out of libfaiss.so and can conflict with/resolve to libcuvs.so symbols, causing RAFT context mismatches. The float CAGRA IDSelector tests (TestGpuIndexCagra) pass because GpuIndexCagra.cu and impl/CuvsCagra.cu are in the hidden visibility list. The binary CAGRA tests fail because they aren't. Buck doesn't have this issue because it links everything statically — there's no DSO boundary. Differential Revision: D101072304
1 parent 57bf474 commit 1f0fffb

4 files changed

Lines changed: 36 additions & 23 deletions

File tree

faiss/gpu/CMakeLists.txt

Lines changed: 11 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -287,32 +287,24 @@ if(FAISS_ENABLE_CUVS)
287287
target_compile_definitions(faiss_avx512 PUBLIC USE_NVIDIA_CUVS=1)
288288
target_compile_definitions(faiss_avx512_spr PUBLIC USE_NVIDIA_CUVS=1)
289289

290-
# Mark all functions as hidden so that we don't generate
291-
# global 'public' functions that also exist in libcuvs.so
290+
# Mark cuVS implementation files as hidden so that cuVS/RAFT
291+
# template instantiations resolve locally within libfaiss.so
292+
# rather than through the PLT to libcuvs.so. This is needed to
293+
# ensure that RAFT cublas resources are created and used within
294+
# the same dynamic library + CUDA runtime context.
292295
#
293-
# This ensures that faiss functions will call the local version
294-
# inside libfaiss.so . This is needed to ensure that things
295-
# like raft cublas resources are created and used within the same
296-
# dynamic library + CUDA runtime context which are requirements
297-
# for valid execution
298-
#
299-
# To still allow these classes to be used by consumers, the
300-
# respective classes/types in the headers are explicitly marked
301-
# as 'public' so they can be used by consumers
296+
# Only impl/utils files are hidden here — they contain the cuVS
297+
# template instantiations and are internal to libfaiss.so.
298+
# Public-facing GpuIndex*.cu files are NOT hidden because their
299+
# headers lack visibility("default") annotations and hiding them
300+
# would break the shared library's public API.
302301
set_source_files_properties(
303-
GpuIndexCagra.cu
304-
GpuDistance.cu
305-
GpuIndexIVFFlat.cu
306-
GpuIndexIVFPQ.cu
307-
GpuIndexFlat.cu
308-
StandardGpuResources.cpp
302+
impl/BinaryCuvsCagra.cu
309303
impl/CuvsCagra.cu
310304
impl/CuvsFlatIndex.cu
311305
impl/CuvsIVFFlat.cu
312306
impl/CuvsIVFPQ.cu
313-
utils/CuvsFilterConvert.cu
314307
utils/CuvsUtils.cu
315-
TARGET_DIRECTORY faiss
316308
PROPERTIES COMPILE_OPTIONS "-fvisibility=hidden")
317309
target_compile_definitions(faiss_gpu_objs PUBLIC USE_NVIDIA_CUVS=1)
318310
endif()

faiss/gpu/impl/BinaryCuvsCagra.cu

Lines changed: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,7 @@
3636

3737
#include <thrust/copy.h>
3838
#include <thrust/device_ptr.h>
39+
#include <thrust/transform.h>
3940

4041
namespace faiss {
4142
namespace gpu {
@@ -284,11 +285,20 @@ void BinaryCuvsCagra::search(
284285
distances_float_view,
285286
filter_ref);
286287

287-
thrust::copy(
288+
// Copy uint32_t indices to idx_t, replacing sentinel values from cuVS
289+
// filtered search with -1. cuVS CAGRA returns large sentinel values
290+
// (e.g., INT32_MAX) for result slots where no valid neighbor was found
291+
// due to filtering. Faiss convention is -1 for "no result".
292+
auto n = n_;
293+
thrust::transform(
288294
raft::resource::get_thrust_policy(raft_handle),
289295
indices_copy.data_handle(),
290296
indices_copy.data_handle() + indices_copy.size(),
291-
indices_view.data_handle());
297+
indices_view.data_handle(),
298+
[n] __device__(uint32_t idx) -> idx_t {
299+
return idx < static_cast<uint32_t>(n) ? static_cast<idx_t>(idx)
300+
: idx_t{-1};
301+
});
292302
auto distances_view = raft::make_device_matrix_view(
293303
outDistances.data(),
294304
static_cast<int64_t>(numQueries),

faiss/gpu/impl/CuvsCagra.cu

Lines changed: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,7 @@
3535

3636
#include <thrust/copy.h>
3737
#include <thrust/device_ptr.h>
38+
#include <thrust/transform.h>
3839
#include <optional>
3940

4041
namespace faiss {
@@ -315,11 +316,20 @@ void CuvsCagra<data_t>::search(
315316
indices_copy.view(),
316317
distances_view,
317318
filter_ref);
318-
thrust::copy(
319+
// Copy uint32_t indices to idx_t, replacing sentinel values from cuVS
320+
// filtered search with -1. cuVS CAGRA returns large sentinel values
321+
// (e.g., INT32_MAX) for result slots where no valid neighbor was found
322+
// due to filtering. Faiss convention is -1 for "no result".
323+
auto n = n_;
324+
thrust::transform(
319325
raft::resource::get_thrust_policy(raft_handle),
320326
indices_copy.data_handle(),
321327
indices_copy.data_handle() + indices_copy.size(),
322-
indices_view.data_handle());
328+
indices_view.data_handle(),
329+
[n] __device__(uint32_t idx) -> idx_t {
330+
return idx < static_cast<uint32_t>(n) ? static_cast<idx_t>(idx)
331+
: idx_t{-1};
332+
});
323333
}
324334

325335
template <typename data_t>

faiss/gpu/utils/CuvsFilterConvert.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -39,3 +39,4 @@ void convert_to_bitset(
3939
cuvs::core::bitset_view<uint32_t, int64_t> bitset,
4040
int num_threads = 0);
4141
} // namespace faiss::gpu
42+
#pragma GCC visibility pop

0 commit comments

Comments
 (0)