Support vulkan backend in build system

.github/workflows/cpp-golang-rust.yml

@@ -132,7 +132,9 @@ jobs:
         CURVE_DIR=icicle-${CURVE//_/-}
         export ICICLE_BACKEND_INSTALL_DIR=${{ steps.cuda-flag.outputs.INSTALL_PATH }}
         cd ./$CURVE_DIR
-        cargo test --release --verbose
+        cargo test --release --verbose -- --skip phase
+        cargo test phase2 --release
+        cargo test phase3 --release
     - name: Run C++ curve Tests
       working-directory: ./icicle/build/tests
       if: needs.check-changed-files.outputs.cpp == 'true'
@@ -260,7 +262,9 @@ jobs:
         FIELD_DIR=icicle-${FIELD//_/-}
         export ICICLE_BACKEND_INSTALL_DIR=${{ steps.cuda-flag.outputs.INSTALL_PATH }}
         cd ./$FIELD_DIR
-        cargo test --release --verbose
+        cargo test --release --verbose -- --skip phase
+        cargo test phase2 --release
+        cargo test phase3 --release
     - name: Run C++ field Tests
       working-directory: ./icicle/build/tests
       if: needs.check-changed-files.outputs.cpp == 'true'

icicle/CMakeLists.txt

@@ -55,11 +55,13 @@ endif()
 set(CMAKE_POSITION_INDEPENDENT_CODE ON)
 
 # Build options
-option(BUILD_TESTS "Build unit tests. Default=OFF" OFF)
+option(BUILD_TESTS "Build unit test2s. Default=OFF" OFF)
+# Backends: typically CPU is built into the frontend, the rest are DSOs loaded at runtime from installation
 option(CPU_BACKEND "Build CPU backend. Default=ON" ON)
-# TODO Yuval: consider decoupling backends from frontend build. Is that worth the effort?
+# To enable building backends, use the following options: (note they are in private repos)
 option(CUDA_BACKEND "Branch/commit to pull for CUDA backend or `local` if under icicle/backend/cuda. Default=OFF" OFF)
 option(METAL_BACKEND "Branch/commit to pull for METAL backend or `local` if under icicle/backend/metal. Default=OFF" OFF)
+option(VULKAN_BACKEND "Branch/commit to pull for VULKAN backend or `local` if under icicle/backend/vulkan. Default=OFF" OFF)
 
 # features that some fields/curves have and some don't.
 option(NTT "Build NTT" ON)
@@ -129,55 +131,8 @@ if (CPU_BACKEND)
   add_subdirectory(backend/cpu)
 endif()
 
-if (CUDA_BACKEND)
-  string(TOLOWER "${CUDA_BACKEND}" CUDA_BACKEND_LOWER)
-  if (CUDA_BACKEND_LOWER STREQUAL "local")
-    # CUDA backend is local, no need to pull
-    message(STATUS "Adding CUDA backend from local path: icicle/backend/cuda")
-    add_subdirectory(backend/cuda)
-
-    # Set the compile definition for the backend build directory
-    add_compile_definitions(BACKEND_BUILD_DIR="${CMAKE_BINARY_DIR}/backend")
-  else()
-    set(CUDA_BACKEND_URL "[email protected]:ingonyama-zk/icicle-cuda-backend.git")
-
-    include(FetchContent)
-    message(STATUS "Fetching cuda backend from ${CUDA_BACKEND_URL}:${CUDA_BACKEND}")
-    FetchContent_Declare(
-      cuda_backend
-      GIT_REPOSITORY ${CUDA_BACKEND_URL}
-      GIT_TAG ${CUDA_BACKEND}
-    )
-    FetchContent_MakeAvailable(cuda_backend)
-    # Set the compile definition for the backend build directory
-    add_compile_definitions(BACKEND_BUILD_DIR="${CMAKE_BINARY_DIR}/_deps/cuda_backend-build")
-    endif()
-endif()
-
-if (METAL_BACKEND)
-  string(TOLOWER "${METAL_BACKEND}" METAL_BACKEND_LOWER)
-  if (METAL_BACKEND_LOWER STREQUAL "local")
-    # METAL backend is local, no need to pull
-    message(STATUS "Adding Metal backend from local path: icicle/backend/metal")
-    add_subdirectory(backend/metal)
-
-    # Set the compile definition for the backend build directory
-    add_compile_definitions(BACKEND_BUILD_DIR="${CMAKE_BINARY_DIR}/backend")
-  else()
-    set(METAL_BACKEND_URL "[email protected]:ingonyama-zk/icicle-metal-backend.git")
-
-    include(FetchContent)
-    message(STATUS "Fetching cuda backend from ${METAL_BACKEND_URL}:${METAL_BACKEND}")
-    FetchContent_Declare(
-      metal_backend
-      GIT_REPOSITORY ${METAL_BACKEND_URL}
-      GIT_TAG ${METAL_BACKEND}
-    )
-    FetchContent_MakeAvailable(metal_backend)
-    # Set the compile definition for the backend build directory
-    add_compile_definitions(BACKEND_BUILD_DIR="${CMAKE_BINARY_DIR}/_deps/metal_backend-build")
-    endif()
-endif()
+# Include and configure (for build) backends based on the backend options
+include(cmake/backend_include.cmake)
 
 if (BUILD_TESTS)
   add_subdirectory(tests)

icicle/cmake/backend_include.cmake

@@ -0,0 +1,74 @@
+if (CUDA_BACKEND)
+  string(TOLOWER "${CUDA_BACKEND}" CUDA_BACKEND_LOWER)
+  if (CUDA_BACKEND_LOWER STREQUAL "local")
+    # CUDA backend is local, no need to pull
+    message(STATUS "Adding CUDA backend from local path: icicle/backend/cuda")
+    add_subdirectory(backend/cuda)
+
+    # Set the compile definition for the backend build directory
+    add_compile_definitions(BACKEND_BUILD_DIR="${CMAKE_BINARY_DIR}/backend")
+  else()
+    set(CUDA_BACKEND_URL "[email protected]:ingonyama-zk/icicle-cuda-backend.git")
+
+    include(FetchContent)
+    message(STATUS "Fetching cuda backend from ${CUDA_BACKEND_URL}:${CUDA_BACKEND}")
+    FetchContent_Declare(
+      cuda_backend
+      GIT_REPOSITORY ${CUDA_BACKEND_URL}
+      GIT_TAG ${CUDA_BACKEND}
+    )
+    FetchContent_MakeAvailable(cuda_backend)
+    # Set the compile definition for the backend build directory
+    add_compile_definitions(BACKEND_BUILD_DIR="${CMAKE_BINARY_DIR}/_deps/cuda_backend-build")
+    endif()
+endif()
+
+if (METAL_BACKEND)
+  string(TOLOWER "${METAL_BACKEND}" METAL_BACKEND_LOWER)
+  if (METAL_BACKEND_LOWER STREQUAL "local")
+    # METAL backend is local, no need to pull
+    message(STATUS "Adding Metal backend from local path: icicle/backend/metal")
+    add_subdirectory(backend/metal)
+
+    # Set the compile definition for the backend build directory
+    add_compile_definitions(BACKEND_BUILD_DIR="${CMAKE_BINARY_DIR}/backend")
+  else()
+    set(METAL_BACKEND_URL "[email protected]:ingonyama-zk/icicle-metal-backend.git")
+
+    include(FetchContent)
+    message(STATUS "Fetching cuda backend from ${METAL_BACKEND_URL}:${METAL_BACKEND}")
+    FetchContent_Declare(
+      metal_backend
+      GIT_REPOSITORY ${METAL_BACKEND_URL}
+      GIT_TAG ${METAL_BACKEND}
+    )
+    FetchContent_MakeAvailable(metal_backend)
+    # Set the compile definition for the backend build directory
+    add_compile_definitions(BACKEND_BUILD_DIR="${CMAKE_BINARY_DIR}/_deps/metal_backend-build")
+    endif()
+endif()
+
+if (VULKAN_BACKEND)
+  string(TOLOWER "${VULKAN_BACKEND}" VULKAN_BACKEND_LOWER)
+  if (VULKAN_BACKEND_LOWER STREQUAL "local")
+    # VULKAN backend is local, no need to pull
+    message(STATUS "Adding Vulkan backend from local path: icicle/backend/vulkan")
+    add_subdirectory(backend/vulkan)
+
+    # Set the compile definition for the backend build directory
+    add_compile_definitions(BACKEND_BUILD_DIR="${CMAKE_BINARY_DIR}/backend")
+  else()
+    set(VULKAN_BACKEND_URL "[email protected]:ingonyama-zk/icicle-vulkan-backend.git")
+
+    include(FetchContent)
+    message(STATUS "Fetching cuda backend from ${VULKAN_BACKEND_URL}:${VULKAN_BACKEND}")
+    FetchContent_Declare(
+      vulkan_backend
+      GIT_REPOSITORY ${VULKAN_BACKEND_URL}
+      GIT_TAG ${VULKAN_BACKEND}
+    )
+    FetchContent_MakeAvailable(vulkan_backend)
+    # Set the compile definition for the backend build directory
+    add_compile_definitions(BACKEND_BUILD_DIR="${CMAKE_BINARY_DIR}/_deps/vulkan-backend-build")
+    endif()
+endif()

icicle/tests/test_device_api.cpp

@@ -27,37 +27,40 @@ TEST_F(DeviceApiTest, MemoryCopySync)
     int output[2] = {0, 0};
 
     icicle::Device dev = {device_type, 0};
-    icicle_set_device(dev);
-
-    void* dev_mem = nullptr;
-    ICICLE_CHECK(icicle_malloc(&dev_mem, sizeof(input)));
-    ICICLE_CHECK(icicle_copy_to_device(dev_mem, input, sizeof(input)));
-    ICICLE_CHECK(icicle_copy_to_host(output, dev_mem, sizeof(input)));
-    ICICLE_CHECK(icicle_free(dev_mem));
-
+    ICICLE_CHECK(icicle_set_device(dev));
+
+    void* dev_memA = nullptr;
+    void* dev_memB = nullptr;
+    // test copy host->device->device->host
+    ICICLE_CHECK(icicle_malloc(&dev_memA, sizeof(input)));
+    ICICLE_CHECK(icicle_malloc(&dev_memB, sizeof(input)));
+    ICICLE_CHECK(icicle_copy_to_device(dev_memA, input, sizeof(input)));
+    ICICLE_CHECK(icicle_copy(dev_memB, dev_memA, sizeof(input)));
+    ICICLE_CHECK(icicle_copy_to_host(output, dev_memB, sizeof(input)));
+    ICICLE_CHECK(icicle_free(dev_memB));
     ASSERT_EQ(0, memcmp(input, output, sizeof(input)));
   }
 }
 
 TEST_F(DeviceApiTest, MemoryCopySyncWithOffset)
 {
   int input[4] = {1, 2, 3, 4};
+  int expected[4] = {3, 4, 1, 2};
 
   for (const auto& device_type : s_registered_devices) {
-    int output[2] = {0, 0};
+    int output[4] = {0, 0, 0, 0};
 
     icicle::Device dev = {device_type, 0};
-    icicle_set_device(dev);
+    ICICLE_CHECK(icicle_set_device(dev));
 
     int* dev_mem = nullptr;
-    ICICLE_CHECK(
-      icicle_malloc((void**)&dev_mem, sizeof(input))); // allocating larger memory to have offset on this buffer to copy
-                                                       // 2 values from offset (that is copy {2,3} only)
-    ICICLE_CHECK(icicle_copy_to_device(dev_mem + 1, input + 1, sizeof(output)));
-    ICICLE_CHECK(icicle_copy_to_host(output, dev_mem + 1, sizeof(output)));
+    ICICLE_CHECK(icicle_malloc((void**)&dev_mem, 4 * sizeof(int)));
+    ICICLE_CHECK(icicle_copy_to_device(dev_mem, input + 2, 2 * sizeof(int)));
+    ICICLE_CHECK(icicle_copy_to_device(dev_mem + 2, input, 2 * sizeof(int)));
+    ICICLE_CHECK(icicle_copy_to_host(output, dev_mem, 4 * sizeof(int)));
     ICICLE_CHECK(icicle_free(dev_mem));
 
-    ASSERT_EQ(0, memcmp(input + 1, output, sizeof(output)));
+    ASSERT_EQ(0, memcmp(expected, output, 4 * sizeof(int)));
   }
 }
 
@@ -68,7 +71,7 @@ TEST_F(DeviceApiTest, MemoryCopyAsync)
     int output[2] = {0, 0};
 
     icicle::Device dev = {device_type, 0};
-    icicle_set_device(dev);
+    ICICLE_CHECK(icicle_set_device(dev));
     void* dev_mem = nullptr;
 
     icicleStreamHandle stream;
@@ -90,7 +93,7 @@ TEST_F(DeviceApiTest, CopyDeviceInference)
     int output[2] = {0, 0};
 
     icicle::Device dev = {device_type, 0};
-    icicle_set_device(dev);
+    ICICLE_CHECK(icicle_set_device(dev));
     void* dev_mem = nullptr;
 
     ICICLE_CHECK(icicle_malloc(&dev_mem, sizeof(input)));
@@ -101,15 +104,14 @@ TEST_F(DeviceApiTest, CopyDeviceInference)
   }
 }
 
-TEST_F(DeviceApiTest, Memest)
+TEST_F(DeviceApiTest, Memset)
 {
   char expected[2] = {1, 2};
   for (const auto& device_type : s_registered_devices) {
     char host_mem[2] = {0, 0};
 
-    // icicle::Device dev = {device_type, 0};
-    icicle::Device dev = {"CPU", 0};
-    icicle_set_device(dev);
+    icicle::Device dev = {device_type, 0};
+    ICICLE_CHECK(icicle_set_device(dev));
     char* dev_mem = nullptr;
 
     ICICLE_CHECK(icicle_malloc((void**)&dev_mem, sizeof(host_mem)));
@@ -125,7 +127,7 @@ TEST_F(DeviceApiTest, ApiError)
 {
   for (const auto& device_type : s_registered_devices) {
     icicle::Device dev = {device_type, 0};
-    icicle_set_device(dev);
+    ICICLE_CHECK(icicle_set_device(dev));
     void* dev_mem = nullptr;
     EXPECT_ANY_THROW(ICICLE_CHECK(icicle_malloc(&dev_mem, -1)));
   }
@@ -137,7 +139,7 @@ TEST_F(DeviceApiTest, AvailableMemory)
   const bool is_cuda_registered = eIcicleError::SUCCESS == icicle_is_device_available(dev);
   if (!is_cuda_registered) { GTEST_SKIP(); } // most devices do not support this
 
-  icicle_set_device(dev);
+  ICICLE_CHECK(icicle_set_device(dev));
   size_t total, free;
   ASSERT_EQ(eIcicleError::SUCCESS, icicle_get_available_memory(total, free));
 
@@ -159,13 +161,13 @@ TEST_F(DeviceApiTest, memoryTracker)
 {
   // need two devices for this test
   if (s_registered_devices.size() == 1) { return; }
-  const int NOF_ALLOCS = 1000;
+  const int NOF_ALLOCS = 200; // Note that some backends have a bound (typically 256) on allocations
   const int ALLOC_SIZE = 1 << 20;
 
   MemoryTracker<Device> tracker{};
   ICICLE_ASSERT(s_main_device != UNKOWN_DEVICE) << "memoryTracker test assumes more than one device";
   Device main_device = {s_main_device, 0};
-  icicle_set_device(main_device);
+  ICICLE_CHECK(icicle_set_device(main_device));
 
   std::vector<void*> allocated_addresses(NOF_ALLOCS, nullptr);
 
@@ -195,12 +197,29 @@ TEST_F(DeviceApiTest, memoryTracker)
   ASSERT_EQ(eIcicleError::INVALID_POINTER, icicle_is_active_device_memory(host_mem.get()));
 
   // test that we still identify correctly after switching device
-  icicle_set_device({"CPU", 0});
+  ICICLE_CHECK(icicle_set_device({"CPU", 0}));
   const void* addr = (void*)((size_t)*allocated_addresses.begin() + rand_uint_32b(0, RAND_MAX) % ALLOC_SIZE);
   ASSERT_EQ(eIcicleError::INVALID_POINTER, icicle_is_active_device_memory(addr));
   ASSERT_EQ(eIcicleError::INVALID_POINTER, icicle_is_active_device_memory(host_mem.get()));
   auto it = tracker.identify(addr);
   ASSERT_EQ(*it->first, main_device);
+
+  ICICLE_CHECK(icicle_set_device(main_device));
+  START_TIMER(remove);
+  for (auto& it : allocated_addresses) {
+    tracker.remove_allocation(it);
+  }
+  END_TIMER_AVERAGE(remove, "memory-tracker: remove average", true, NOF_ALLOCS);
+
+  START_TIMER(free);
+  for (auto& it : allocated_addresses) {
+    icicle_free(it);
+  }
+  END_TIMER_AVERAGE(free, "memory-tracker: free average", true, NOF_ALLOCS);
+
+  void* mem;
+  ICICLE_CHECK(icicle_malloc(&mem, ALLOC_SIZE));
+  ICICLE_CHECK(icicle_free(mem));
 }
 
 int main(int argc, char** argv)

scripts/pull_metal_backend.sh

@@ -14,8 +14,6 @@ ABS_METAL_DIR=$(realpath ${BACKEND_DIR})/metal
 
 echo "Trying to pull Metal backend commit '${METAL_BACKEND}' to '${ABS_METAL_DIR}'"
 
-exit 1
-
 if [ -d "${ABS_METAL_DIR}" ] && [ "$(ls -A ${ABS_METAL_DIR})" ]; then
     echo "Directory ${ABS_METAL_DIR} is not empty."
     read -p "Do you want to proceed with fetching and resetting? (y/n): " response

scripts/pull_vulkan_backend.sh

@@ -0,0 +1,42 @@
+#!/bin/bash
+
+VULKAN_BACKEND=${1:-main}
+BACKEND_DIR=${2:-./icicle/backend}
+
+# Check if BACKEND_DIR exists
+if [ ! -d "${BACKEND_DIR}" ]; then
+    echo "Error: Directory '${BACKEND_DIR}' does not exist."
+    exit 1
+fi
+
+# Get the absolute path of the backend directory
+ABS_VULKAN_DIR=$(realpath ${BACKEND_DIR})/vulkan
+
+echo "Trying to pull vulkan backend commit '${VULKAN_BACKEND}' to '${ABS_VULKAN_DIR}'"
+
+if [ -d "${ABS_VULKAN_DIR}" ] && [ "$(ls -A ${ABS_VULKAN_DIR})" ]; then
+    echo "Directory ${ABS_VULKAN_DIR} is not empty."
+    read -p "Do you want to proceed with fetching and resetting? (y/n): " response
+    case "$response" in
+        [Yy]* )
+            echo "Proceeding with fetch and reset..."
+            cd ${ABS_VULKAN_DIR}
+            git fetch origin
+            git reset --hard origin/${VULKAN_BACKEND}
+            ;;
+        [Nn]* )
+            echo "Aborting."
+            exit 1
+            ;;
+        * )
+            echo "Invalid input. Aborting."
+            exit 1
+            ;;
+    esac
+else
+    echo "Directory ${ABS_VULKAN_DIR} is empty or does not exist. Cloning..."
+    mkdir -p ${ABS_VULKAN_DIR}
+    cd ${ABS_VULKAN_DIR}
+    git clone [email protected]:ingonyama-zk/icicle-vulkan-backend.git ${ABS_VULKAN_DIR}
+    git checkout ${VULKAN_BACKEND}
+fi