Skip to content

Commit

Permalink
apacheGH-37523: [C++][CI][CUDA] Don't use newer API and add missing C…
Browse files Browse the repository at this point in the history
…UDA dependencies (apache#37497)

### Rationale for this change
apache#37365 was built locally using a newer version of the CUDA driver API than the crossbow builds run with causing the crossbow build to fail due to a change in macro names. This puts the macro name back to allow it to build with the older version of the cuda driver api.

* Closes: apache#37523

Authored-by: Matt Topol <[email protected]>
Signed-off-by: Matt Topol <[email protected]>
  • Loading branch information
zeroshade authored and loicalleyne committed Nov 13, 2023
1 parent b9efba5 commit e1e951c
Show file tree
Hide file tree
Showing 7 changed files with 19 additions and 15 deletions.
4 changes: 1 addition & 3 deletions cpp/src/arrow/c/bridge_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1222,9 +1222,7 @@ class MyDevice : public Device {

virtual ~MySyncEvent() = default;
Status Wait() override { return Status::OK(); }
Status Record(const Device::Stream&, const unsigned int) override {
return Status::OK();
}
Status Record(const Device::Stream&) override { return Status::OK(); }
};

protected:
Expand Down
4 changes: 1 addition & 3 deletions cpp/src/arrow/device.h
Original file line number Diff line number Diff line change
Expand Up @@ -166,11 +166,9 @@ class ARROW_EXPORT Device : public std::enable_shared_from_this<Device>,
/// @brief Block until sync event is completed.
virtual Status Wait() = 0;

inline Status Record(const Stream& st) { return Record(st, 0); }

/// @brief Record the wrapped event on the stream so it triggers
/// the event when the stream gets to that point in its queue.
virtual Status Record(const Stream&, const unsigned int flags) = 0;
virtual Status Record(const Stream&) = 0;

protected:
/// If creating this with a passed in event, the caller must ensure
Expand Down
4 changes: 4 additions & 0 deletions cpp/src/arrow/gpu/ArrowCUDAConfig.cmake.in
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,10 @@ include(CMakeFindDependencyMacro)
find_dependency(Arrow)
if(CMAKE_VERSION VERSION_LESS 3.17)
find_package(CUDA REQUIRED)
add_library(ArrowCUDA::cuda_driver SHARED IMPORTED)
set_target_properties(ArrowCUDA::cuda_driver
PROPERTIES IMPORTED_LOCATION "${CUDA_CUDA_LIBRARY}"
INTERFACE_INCLUDE_DIRECTORIES "${CUDA_INCLUDE_DIRS}")
else()
find_package(CUDAToolkit REQUIRED)
endif()
Expand Down
8 changes: 6 additions & 2 deletions cpp/src/arrow/gpu/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -32,8 +32,11 @@ endif()
set(ARROW_CUDA_LINK_LIBS arrow::flatbuffers)
if(CMAKE_VERSION VERSION_LESS 3.17)
find_package(CUDA REQUIRED)
set(ARROW_CUDA_SHARED_LINK_LIBS ${CUDA_CUDA_LIBRARY})
include_directories(SYSTEM ${CUDA_INCLUDE_DIRS})
add_library(ArrowCUDA::cuda_driver SHARED IMPORTED)
set_target_properties(ArrowCUDA::cuda_driver
PROPERTIES IMPORTED_LOCATION "${CUDA_CUDA_LIBRARY}"
INTERFACE_INCLUDE_DIRECTORIES "${CUDA_INCLUDE_DIRS}")
set(ARROW_CUDA_SHARED_LINK_LIBS ArrowCUDA::cuda_driver)
else()
# find_package(CUDA) is deprecated, and for newer CUDA, it doesn't
# recognize that the CUDA driver library is in the "stubs" dir, but
Expand Down Expand Up @@ -61,6 +64,7 @@ add_arrow_lib(arrow_cuda
${ARROW_CUDA_SHARED_LINK_LIBS}
SHARED_INSTALL_INTERFACE_LIBS
Arrow::arrow_shared
${ARROW_CUDA_SHARED_LINK_LIBS}
# Static arrow_cuda must also link against CUDA shared libs
STATIC_LINK_LIBS
${ARROW_CUDA_LINK_LIBS}
Expand Down
2 changes: 1 addition & 1 deletion cpp/src/arrow/gpu/arrow-cuda.pc.in
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,6 @@ libdir=@ARROW_PKG_CONFIG_LIBDIR@
Name: Apache Arrow CUDA
Description: CUDA integration library for Apache Arrow
Version: @ARROW_VERSION@
Requires: arrow
Requires: arrow cuda
Libs: -L${libdir} -larrow_cuda
Cflags: -I${includedir}
10 changes: 5 additions & 5 deletions cpp/src/arrow/gpu/cuda_context.cc
Original file line number Diff line number Diff line change
Expand Up @@ -322,8 +322,9 @@ Status CudaDevice::Stream::WaitEvent(const Device::SyncEvent& event) {
}

ContextSaver set_temporary(reinterpret_cast<CUcontext>(context_.get()->handle()));
CU_RETURN_NOT_OK("cuStreamWaitEvent",
cuStreamWaitEvent(value(), cu_event, CU_EVENT_WAIT_DEFAULT));
// we are currently building with CUDA toolkit 11.0.3 which doesn't have enum
// values for the flags yet. The "flags" param *must* be 0 for now.
CU_RETURN_NOT_OK("cuStreamWaitEvent", cuStreamWaitEvent(value(), cu_event, 0));
return Status::OK();
}

Expand All @@ -339,15 +340,14 @@ Status CudaDevice::SyncEvent::Wait() {
return Status::OK();
}

Status CudaDevice::SyncEvent::Record(const Device::Stream& st, const unsigned int flags) {
Status CudaDevice::SyncEvent::Record(const Device::Stream& st) {
auto cuda_stream = checked_cast<const CudaDevice::Stream*, const Device::Stream*>(&st);
if (!cuda_stream) {
return Status::Invalid("CudaDevice::Event cannot record on non-cuda stream");
}

ContextSaver set_temporary(reinterpret_cast<CUcontext>(context_.get()->handle()));
CU_RETURN_NOT_OK("cuEventRecordWithFlags",
cuEventRecordWithFlags(value(), cuda_stream->value(), flags));
CU_RETURN_NOT_OK("cuEventRecord", cuEventRecord(value(), cuda_stream->value()));
return Status::OK();
}

Expand Down
2 changes: 1 addition & 1 deletion cpp/src/arrow/gpu/cuda_context.h
Original file line number Diff line number Diff line change
Expand Up @@ -212,7 +212,7 @@ class ARROW_EXPORT CudaDevice : public Device {
///
/// Once the stream completes the tasks previously added to it,
/// it will trigger the event.
Status Record(const Device::Stream&, const unsigned int) override;
Status Record(const Device::Stream&) override;

protected:
friend class CudaMemoryManager;
Expand Down

0 comments on commit e1e951c

Please sign in to comment.