From 13f0cd8ee44833efbced05c8505de12f9f02f235 Mon Sep 17 00:00:00 2001 From: Matt Topol Date: Tue, 5 Sep 2023 11:33:18 -0400 Subject: [PATCH] GH-37523: [C++][CI][CUDA] Don't use newer API and add missing CUDA dependencies (#37497) ### Rationale for this change #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: #37523 Authored-by: Matt Topol Signed-off-by: Matt Topol --- cpp/src/arrow/c/bridge_test.cc | 4 +--- cpp/src/arrow/device.h | 4 +--- cpp/src/arrow/gpu/ArrowCUDAConfig.cmake.in | 4 ++++ cpp/src/arrow/gpu/CMakeLists.txt | 8 ++++++-- cpp/src/arrow/gpu/arrow-cuda.pc.in | 2 +- cpp/src/arrow/gpu/cuda_context.cc | 10 +++++----- cpp/src/arrow/gpu/cuda_context.h | 2 +- 7 files changed, 19 insertions(+), 15 deletions(-) diff --git a/cpp/src/arrow/c/bridge_test.cc b/cpp/src/arrow/c/bridge_test.cc index 2aedccc965c45..bd0e498a9f332 100644 --- a/cpp/src/arrow/c/bridge_test.cc +++ b/cpp/src/arrow/c/bridge_test.cc @@ -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: diff --git a/cpp/src/arrow/device.h b/cpp/src/arrow/device.h index 066ca7e32a4fe..efb0a5ab400a1 100644 --- a/cpp/src/arrow/device.h +++ b/cpp/src/arrow/device.h @@ -166,11 +166,9 @@ class ARROW_EXPORT Device : public std::enable_shared_from_this, /// @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 diff --git a/cpp/src/arrow/gpu/ArrowCUDAConfig.cmake.in b/cpp/src/arrow/gpu/ArrowCUDAConfig.cmake.in index bb36abf241193..626c536a08d5e 100644 --- a/cpp/src/arrow/gpu/ArrowCUDAConfig.cmake.in +++ b/cpp/src/arrow/gpu/ArrowCUDAConfig.cmake.in @@ -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() diff --git a/cpp/src/arrow/gpu/CMakeLists.txt b/cpp/src/arrow/gpu/CMakeLists.txt index 2556f30f0d5e6..a5b176793e495 100644 --- a/cpp/src/arrow/gpu/CMakeLists.txt +++ b/cpp/src/arrow/gpu/CMakeLists.txt @@ -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 @@ -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} diff --git a/cpp/src/arrow/gpu/arrow-cuda.pc.in b/cpp/src/arrow/gpu/arrow-cuda.pc.in index 84c8c5889856e..173d7d91ef9b5 100644 --- a/cpp/src/arrow/gpu/arrow-cuda.pc.in +++ b/cpp/src/arrow/gpu/arrow-cuda.pc.in @@ -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} diff --git a/cpp/src/arrow/gpu/cuda_context.cc b/cpp/src/arrow/gpu/cuda_context.cc index fb8935319ed1c..81542d339bd70 100644 --- a/cpp/src/arrow/gpu/cuda_context.cc +++ b/cpp/src/arrow/gpu/cuda_context.cc @@ -322,8 +322,9 @@ Status CudaDevice::Stream::WaitEvent(const Device::SyncEvent& event) { } ContextSaver set_temporary(reinterpret_cast(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(); } @@ -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(&st); if (!cuda_stream) { return Status::Invalid("CudaDevice::Event cannot record on non-cuda stream"); } ContextSaver set_temporary(reinterpret_cast(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(); } diff --git a/cpp/src/arrow/gpu/cuda_context.h b/cpp/src/arrow/gpu/cuda_context.h index e4d8482855f6b..56c4f3203524f 100644 --- a/cpp/src/arrow/gpu/cuda_context.h +++ b/cpp/src/arrow/gpu/cuda_context.h @@ -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;