From 702855a27d215c2c411ada32f2cc1da5c186c7d3 Mon Sep 17 00:00:00 2001 From: Hayodea Hekol Date: Sun, 7 Dec 2025 19:12:26 -0400 Subject: [PATCH] OClCollMeshEngn: Use uniq_ptr for Cl handle RAII We no longer use a goto slide to deal with initialization errors in setup()/finalize(). We use RAII instead. --- .../openClCollatingAndMeshingEngine.cpp | 173 ++++++++++-------- .../openClCollatingAndMeshingEngine.h | 36 +++- 2 files changed, 125 insertions(+), 84 deletions(-) diff --git a/stimBuffApis/livoxGen1/openClCollatingAndMeshingEngine.cpp b/stimBuffApis/livoxGen1/openClCollatingAndMeshingEngine.cpp index 9562cde..024b90b 100644 --- a/stimBuffApis/livoxGen1/openClCollatingAndMeshingEngine.cpp +++ b/stimBuffApis/livoxGen1/openClCollatingAndMeshingEngine.cpp @@ -33,8 +33,6 @@ OpenClCollatingAndMeshingEngine::OpenClCollatingAndMeshingEngine( PcloudStimulusProducer& parent_) : parent(parent_), computeDevice(nullptr), -slotCompactorProgram(nullptr), collateProgram(nullptr), -slotCompactorKernel(nullptr), collateKernel(nullptr), clAssemblyBufferClBuffer(nullptr), clCollationBufferClBuffer(nullptr), clAverageIntensityBufferClBuffer(nullptr), @@ -82,8 +80,8 @@ bool OpenClCollatingAndMeshingEngine::setup() } // Get ComputeDevice from smo hooks - computeDevice = smoHooksPtr->ComputeManager_getDevice(); - if (!computeDevice) + auto wip_computeDevice = smoHooksPtr->ComputeManager_getDevice(); + if (!wip_computeDevice) { std::cerr << __func__ << ": failed to get compute device" << std::endl; return false; @@ -109,7 +107,7 @@ bool OpenClCollatingAndMeshingEngine::setup() if (!frameAssemblyDesc || frameAssemblyDesc->slots.empty()) { std::cerr << __func__ << ": invalid frame descriptor" << std::endl; - goto cleanup; + return false; } // Create OpenCL buffers using smo hooks @@ -117,71 +115,90 @@ bool OpenClCollatingAndMeshingEngine::setup() { std::cerr << __func__ << ": createUseHostPtrBuffer hook not available" << std::endl; - goto cleanup; + return false; } - clAssemblyBufferClBuffer = smoHooksPtr + auto wip_clAssemblyBufferClBuffer = smoHooksPtr ->ComputeManager_createUseHostPtrBuffer( assemblyBufferPtr, assemblyBufferSize, CL_MEM_READ_WRITE); - if (!clAssemblyBufferClBuffer) + if (!wip_clAssemblyBufferClBuffer) { std::cerr << __func__ << ": failed to create assembly buffer" << std::endl; - goto cleanup; + return false; } - clCollationBufferClBuffer = smoHooksPtr + auto wip_clCollationBufferClBuffer = smoHooksPtr ->ComputeManager_createUseHostPtrBuffer( collationBufferPtr, collationBufferSize, CL_MEM_WRITE_ONLY); - if (!clCollationBufferClBuffer) + if (!wip_clCollationBufferClBuffer) { std::cerr << __func__ << ": failed to create collation buffer" << std::endl; - goto cleanup; + return false; } - clAverageIntensityBufferClBuffer = smoHooksPtr + auto wip_clAverageIntensityBufferClBuffer = smoHooksPtr ->ComputeManager_createUseHostPtrBuffer( averageIntensityBufferPtr, averageIntensityBufferSize, CL_MEM_WRITE_ONLY); - if (!clAverageIntensityBufferClBuffer) + if (!wip_clAverageIntensityBufferClBuffer) { std::cerr << __func__ << ": failed to create average intensity buffer" << std::endl; - goto cleanup; + return false; } // Cache cl_mem handles for the device we're using - clAssemblyBuffer = clAssemblyBufferClBuffer - ->getAssociatedBufferHandleForDevice(computeDevice); - clCollationBuffer = clCollationBufferClBuffer - ->getAssociatedBufferHandleForDevice(computeDevice); - clAverageIntensityBuffer = clAverageIntensityBufferClBuffer - ->getAssociatedBufferHandleForDevice(computeDevice); + cl_mem wip_clAssemblyBuffer = wip_clAssemblyBufferClBuffer + ->getAssociatedBufferHandleForDevice(wip_computeDevice); + cl_mem wip_clCollationBuffer = wip_clCollationBufferClBuffer + ->getAssociatedBufferHandleForDevice(wip_computeDevice); + cl_mem wip_clAverageIntensityBuffer = wip_clAverageIntensityBufferClBuffer + ->getAssociatedBufferHandleForDevice(wip_computeDevice); - if (!clAssemblyBuffer || !clCollationBuffer || !clAverageIntensityBuffer) + if (!wip_clAssemblyBuffer || !wip_clCollationBuffer + || !wip_clAverageIntensityBuffer) { std::cerr << __func__ << ": failed to get buffer handles for device" << std::endl; - goto cleanup; + return false; } // Compile and prepare both kernels - if (!compileAndPrepareKernels()) { - goto cleanup; + ClProgramPtr wip_slotCompactorProgram; + ClProgramPtr wip_collateProgram; + ClKernelPtr wip_slotCompactorKernel; + ClKernelPtr wip_collateKernel; + + if (!compileAndPrepareKernels( + wip_computeDevice, + wip_slotCompactorProgram, wip_collateProgram, + wip_slotCompactorKernel, wip_collateKernel)) + { + return false; } + // All operations succeeded - assign to member variables + computeDevice = wip_computeDevice; + clAssemblyBufferClBuffer = wip_clAssemblyBufferClBuffer; + clCollationBufferClBuffer = wip_clCollationBufferClBuffer; + clAverageIntensityBufferClBuffer = wip_clAverageIntensityBufferClBuffer; + clAssemblyBuffer = wip_clAssemblyBuffer; + clCollationBuffer = wip_clCollationBuffer; + clAverageIntensityBuffer = wip_clAverageIntensityBuffer; + slotCompactorProgram = std::move(wip_slotCompactorProgram); + collateProgram = std::move(wip_collateProgram); + slotCompactorKernel = std::move(wip_slotCompactorKernel); + collateKernel = std::move(wip_collateKernel); + clFlush(computeDevice->commandQueue); clFinish(computeDevice->commandQueue); shouldAcceptRequests = true; return true; - -cleanup: - finalize(); - return false; } void OpenClCollatingAndMeshingEngine::finalize() @@ -261,29 +278,11 @@ void OpenClCollatingAndMeshingEngine::finalize() clAverageIntensityBuffer = nullptr; clAssemblyBuffer = nullptr; - // Release kernels - if (slotCompactorKernel) - { - clReleaseKernel(slotCompactorKernel); - slotCompactorKernel = nullptr; - } - if (collateKernel) - { - clReleaseKernel(collateKernel); - collateKernel = nullptr; - } - - // Release programs - if (slotCompactorProgram) - { - clReleaseProgram(slotCompactorProgram); - slotCompactorProgram = nullptr; - } - if (collateProgram) - { - clReleaseProgram(collateProgram); - collateProgram = nullptr; - } + // Release kernels and programs (handled automatically by unique_ptr destructors) + slotCompactorKernel.reset(); + collateKernel.reset(); + slotCompactorProgram.reset(); + collateProgram.reset(); // Release compute device via smo hooks if (smoHooksPtr && smoHooksPtr->ComputeManager_releaseDevice @@ -385,7 +384,7 @@ bool OpenClCollatingAndMeshingEngine::startCompactKernel( unmapAssemblyBuffer(); bool success = startKernel( - slotCompactorKernel, + slotCompactorKernel.get(), ¤tCompactKernelEvent, setupArgs, validateBuffers, @@ -491,7 +490,7 @@ bool OpenClCollatingAndMeshingEngine::startCollateKernel( size_t globalWorkSize = static_cast(frameAssemblyDesc->numSlots); bool success = startKernel( - collateKernel, + collateKernel.get(), ¤tCollateKernelEvent, setupArgs, validateBuffers, @@ -505,24 +504,28 @@ bool OpenClCollatingAndMeshingEngine::startCollateKernel( } bool OpenClCollatingAndMeshingEngine::compileAndPrepareKernel( + const std::shared_ptr& computeDevice, const char* kernelSource, size_t kernelSourceLen, - const char* kernelName, cl_program& program, cl_kernel& kernel) + const char* kernelName, ClProgramPtr& program, ClKernelPtr& kernel) { cl_int err; // Create program from source - program = clCreateProgramWithSource( + cl_program rawProgram = clCreateProgramWithSource( computeDevice->context, 1, &kernelSource, &kernelSourceLen, &err); - if (err != CL_SUCCESS || !program) + if (err != CL_SUCCESS || !rawProgram) { std::cerr << __func__ << ": failed to create " << kernelName << " program: " << err << std::endl; return false; } + // Wrap in unique_ptr + program = ClProgramPtr(rawProgram); + // Build program - err = clBuildProgram(program, 1, &computeDevice->device, + err = clBuildProgram(program.get(), 1, &computeDevice->device, nullptr, nullptr, nullptr); if (err != CL_SUCCESS) @@ -533,14 +536,14 @@ bool OpenClCollatingAndMeshingEngine::compileAndPrepareKernel( // Print build log if available size_t logSize = 0; clGetProgramBuildInfo( - program, computeDevice->device, CL_PROGRAM_BUILD_LOG, + program.get(), computeDevice->device, CL_PROGRAM_BUILD_LOG, 0, nullptr, &logSize); if (logSize > 0) { std::vector log(logSize); clGetProgramBuildInfo( - program, computeDevice->device, CL_PROGRAM_BUILD_LOG, + program.get(), computeDevice->device, CL_PROGRAM_BUILD_LOG, logSize, log.data(), nullptr); std::cerr << kernelName << " build log: " << log.data() << std::endl; @@ -550,22 +553,28 @@ bool OpenClCollatingAndMeshingEngine::compileAndPrepareKernel( } // Create kernel - kernel = clCreateKernel(program, kernelName, &err); - if (err != CL_SUCCESS || !kernel) + cl_kernel rawKernel = clCreateKernel(program.get(), kernelName, &err); + if (err != CL_SUCCESS || !rawKernel) { std::cerr << __func__ << ": failed to create " << kernelName << " kernel: " << err << std::endl; return false; } + // Wrap in unique_ptr + kernel = ClKernelPtr(rawKernel); + return true; } -bool OpenClCollatingAndMeshingEngine::compileAndPrepareKernels() +bool OpenClCollatingAndMeshingEngine::compileAndPrepareKernels( + const std::shared_ptr& computeDevice, + ClProgramPtr& slotCompactorProgram, ClProgramPtr& collateProgram, + ClKernelPtr& slotCompactorKernel, ClKernelPtr& collateKernel) { // Compile slotCompactor kernel if (!compileAndPrepareKernel( - slotCompactorKernelStart, slotCompactorKernelNBytes, + computeDevice, slotCompactorKernelStart, slotCompactorKernelNBytes, "slotCompactor", slotCompactorProgram, slotCompactorKernel)) { return false; @@ -573,7 +582,7 @@ bool OpenClCollatingAndMeshingEngine::compileAndPrepareKernels() // Compile collateDgrams kernel if (!compileAndPrepareKernel( - collateKernelStart, collateKernelNBytes, + computeDevice, collateKernelStart, collateKernelNBytes, "collate", collateProgram, collateKernel)) { return false; @@ -594,7 +603,7 @@ bool OpenClCollatingAndMeshingEngine::setupSlotCompactorsArgs( // Set kernel arguments for slotCompactor cl_int err; err = clSetKernelArg( - slotCompactorKernel, 0, sizeof(cl_mem), &clAssemblyBuffer); + slotCompactorKernel.get(), 0, sizeof(cl_mem), &clAssemblyBuffer); if (err != CL_SUCCESS) { @@ -603,7 +612,8 @@ bool OpenClCollatingAndMeshingEngine::setupSlotCompactorsArgs( return false; } - err = clSetKernelArg(slotCompactorKernel, 1, sizeof(uint32_t), &numSlots); + err = clSetKernelArg( + slotCompactorKernel.get(), 1, sizeof(uint32_t), &numSlots); if (err != CL_SUCCESS) { std::cerr << __func__ << ": failed to set kernel arg 1: " << err @@ -611,7 +621,8 @@ bool OpenClCollatingAndMeshingEngine::setupSlotCompactorsArgs( return false; } - err = clSetKernelArg(slotCompactorKernel, 2, sizeof(uint32_t), &slotStride); + err = clSetKernelArg( + slotCompactorKernel.get(), 2, sizeof(uint32_t), &slotStride); if (err != CL_SUCCESS) { std::cerr << __func__ << ": failed to set kernel arg 2: " << err @@ -619,7 +630,8 @@ bool OpenClCollatingAndMeshingEngine::setupSlotCompactorsArgs( return false; } - err = clSetKernelArg(slotCompactorKernel, 3, sizeof(uint32_t), &slotSize); + err = clSetKernelArg( + slotCompactorKernel.get(), 3, sizeof(uint32_t), &slotSize); if (err != CL_SUCCESS) { std::cerr << __func__ << ": failed to set kernel arg 3: " << err @@ -628,7 +640,7 @@ bool OpenClCollatingAndMeshingEngine::setupSlotCompactorsArgs( } err = clSetKernelArg( - slotCompactorKernel, 4, sizeof(uint32_t), &nSucceededUint); + slotCompactorKernel.get(), 4, sizeof(uint32_t), &nSucceededUint); if (err != CL_SUCCESS) { @@ -662,7 +674,8 @@ bool OpenClCollatingAndMeshingEngine::setupCollateDgramsArgs( // Set kernel arguments for collateDgrams cl_int err; - err = clSetKernelArg(collateKernel, 0, sizeof(cl_mem), &clAssemblyBuffer); + err = clSetKernelArg( + collateKernel.get(), 0, sizeof(cl_mem), &clAssemblyBuffer); if (err != CL_SUCCESS) { std::cerr << __func__ << ": failed to set kernel arg 0: " << err @@ -670,7 +683,8 @@ bool OpenClCollatingAndMeshingEngine::setupCollateDgramsArgs( return false; } - err = clSetKernelArg(collateKernel, 1, sizeof(cl_mem), &clCollationBuffer); + err = clSetKernelArg( + collateKernel.get(), 1, sizeof(cl_mem), &clCollationBuffer); if (err != CL_SUCCESS) { std::cerr << __func__ << ": failed to set kernel arg 1: " << err @@ -685,7 +699,8 @@ bool OpenClCollatingAndMeshingEngine::setupCollateDgramsArgs( intensityClBuffer = intensityStimFrame->get().clBuffer ->getAssociatedBufferHandleForDevice(computeDevice); } - err = clSetKernelArg(collateKernel, 2, sizeof(cl_mem), &intensityClBuffer); + err = clSetKernelArg( + collateKernel.get(), 2, sizeof(cl_mem), &intensityClBuffer); if (err != CL_SUCCESS) { std::cerr << __func__ << ": failed to set kernel arg 2: " << err @@ -707,7 +722,7 @@ bool OpenClCollatingAndMeshingEngine::setupCollateDgramsArgs( averageIntensityClBuffer = clAverageIntensityBuffer; } err = clSetKernelArg( - collateKernel, 3, sizeof(cl_mem), &averageIntensityClBuffer); + collateKernel.get(), 3, sizeof(cl_mem), &averageIntensityClBuffer); if (err != CL_SUCCESS) { @@ -716,7 +731,7 @@ bool OpenClCollatingAndMeshingEngine::setupCollateDgramsArgs( return false; } - err = clSetKernelArg(collateKernel, 4, sizeof(uint32_t), &slotStride); + err = clSetKernelArg(collateKernel.get(), 4, sizeof(uint32_t), &slotStride); if (err != CL_SUCCESS) { std::cerr << __func__ << ": failed to set kernel arg 4: " << err @@ -724,7 +739,8 @@ bool OpenClCollatingAndMeshingEngine::setupCollateDgramsArgs( return false; } - err = clSetKernelArg(collateKernel, 5, sizeof(uint32_t), &nPointsPerSlot); + err = clSetKernelArg( + collateKernel.get(), 5, sizeof(uint32_t), &nPointsPerSlot); if (err != CL_SUCCESS) { std::cerr << __func__ << ": failed to set kernel arg 5: " << err @@ -732,7 +748,8 @@ bool OpenClCollatingAndMeshingEngine::setupCollateDgramsArgs( return false; } - err = clSetKernelArg(collateKernel, 6, sizeof(uint32_t), &nDgramsPerFrame); + err = clSetKernelArg( + collateKernel.get(), 6, sizeof(uint32_t), &nDgramsPerFrame); if (err != CL_SUCCESS) { std::cerr << __func__ << ": failed to set kernel arg 6: " << err @@ -1202,7 +1219,7 @@ public: } } -#if 0 +#if 1 // Print all averages above thresholds from average intensity buffer if (context->ambienceStimFrame.has_value()) { diff --git a/stimBuffApis/livoxGen1/openClCollatingAndMeshingEngine.h b/stimBuffApis/livoxGen1/openClCollatingAndMeshingEngine.h index 677809f..e6978ef 100644 --- a/stimBuffApis/livoxGen1/openClCollatingAndMeshingEngine.h +++ b/stimBuffApis/livoxGen1/openClCollatingAndMeshingEngine.h @@ -5,6 +5,7 @@ #include #include #include +#include #include #include #include @@ -26,6 +27,25 @@ namespace smo { namespace stim_buff { +// Custom deleters for OpenCL handles +struct ClProgramDeleter +{ + void operator()(cl_program prog) const + { if (prog) { clReleaseProgram(prog); } } +}; + +struct ClKernelDeleter +{ + void operator()(cl_kernel kernel) const + { if (kernel) { clReleaseKernel(kernel); } } +}; + +// Type aliases for OpenCL handle unique_ptrs +using ClProgramPtr = std::unique_ptr< + std::remove_pointer_t, ClProgramDeleter>; +using ClKernelPtr = std::unique_ptr< + std::remove_pointer_t, ClKernelDeleter>; + class PcloudStimulusProducer; class OpenClCollatingAndMeshingEngine @@ -90,10 +110,10 @@ private: // OpenCL infrastructure (managed by ComputeManager) std::shared_ptr computeDevice; - cl_program slotCompactorProgram; - cl_program collateProgram; - cl_kernel slotCompactorKernel; - cl_kernel collateKernel; + ClProgramPtr slotCompactorProgram; + ClProgramPtr collateProgram; + ClKernelPtr slotCompactorKernel; + ClKernelPtr collateKernel; // OpenCL buffers (managed by ComputeManager) std::shared_ptr clAssemblyBufferClBuffer; @@ -145,9 +165,13 @@ private: // Private helper methods bool compileAndPrepareKernel( + const std::shared_ptr& computeDevice, const char* kernelSource, size_t kernelSourceLen, - const char* kernelName, cl_program& program, cl_kernel& kernel); - bool compileAndPrepareKernels(); + const char* kernelName, ClProgramPtr& program, ClKernelPtr& kernel); + bool compileAndPrepareKernels( + const std::shared_ptr& computeDevice, + ClProgramPtr& slotCompactorProgram, ClProgramPtr& collateProgram, + ClKernelPtr& slotCompactorKernel, ClKernelPtr& collateKernel); bool setupSlotCompactorsArgs( StagingBuffer& assemblyBuff, uint32_t nSucceeded); bool setupCollateDgramsArgs(