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.
This commit is contained in:
2025-12-07 19:12:26 -04:00
parent dc5587bfcc
commit 702855a27d
2 changed files with 125 additions and 84 deletions
@@ -33,8 +33,6 @@ OpenClCollatingAndMeshingEngine::OpenClCollatingAndMeshingEngine(
PcloudStimulusProducer& parent_) PcloudStimulusProducer& parent_)
: parent(parent_), : parent(parent_),
computeDevice(nullptr), computeDevice(nullptr),
slotCompactorProgram(nullptr), collateProgram(nullptr),
slotCompactorKernel(nullptr), collateKernel(nullptr),
clAssemblyBufferClBuffer(nullptr), clAssemblyBufferClBuffer(nullptr),
clCollationBufferClBuffer(nullptr), clCollationBufferClBuffer(nullptr),
clAverageIntensityBufferClBuffer(nullptr), clAverageIntensityBufferClBuffer(nullptr),
@@ -82,8 +80,8 @@ bool OpenClCollatingAndMeshingEngine::setup()
} }
// Get ComputeDevice from smo hooks // Get ComputeDevice from smo hooks
computeDevice = smoHooksPtr->ComputeManager_getDevice(); auto wip_computeDevice = smoHooksPtr->ComputeManager_getDevice();
if (!computeDevice) if (!wip_computeDevice)
{ {
std::cerr << __func__ << ": failed to get compute device" << std::endl; std::cerr << __func__ << ": failed to get compute device" << std::endl;
return false; return false;
@@ -109,7 +107,7 @@ bool OpenClCollatingAndMeshingEngine::setup()
if (!frameAssemblyDesc || frameAssemblyDesc->slots.empty()) if (!frameAssemblyDesc || frameAssemblyDesc->slots.empty())
{ {
std::cerr << __func__ << ": invalid frame descriptor" << std::endl; std::cerr << __func__ << ": invalid frame descriptor" << std::endl;
goto cleanup; return false;
} }
// Create OpenCL buffers using smo hooks // Create OpenCL buffers using smo hooks
@@ -117,71 +115,90 @@ bool OpenClCollatingAndMeshingEngine::setup()
{ {
std::cerr << __func__ << ": createUseHostPtrBuffer hook not available" std::cerr << __func__ << ": createUseHostPtrBuffer hook not available"
<< std::endl; << std::endl;
goto cleanup; return false;
} }
clAssemblyBufferClBuffer = smoHooksPtr auto wip_clAssemblyBufferClBuffer = smoHooksPtr
->ComputeManager_createUseHostPtrBuffer( ->ComputeManager_createUseHostPtrBuffer(
assemblyBufferPtr, assemblyBufferSize, CL_MEM_READ_WRITE); assemblyBufferPtr, assemblyBufferSize, CL_MEM_READ_WRITE);
if (!clAssemblyBufferClBuffer) if (!wip_clAssemblyBufferClBuffer)
{ {
std::cerr << __func__ << ": failed to create assembly buffer" std::cerr << __func__ << ": failed to create assembly buffer"
<< std::endl; << std::endl;
goto cleanup; return false;
} }
clCollationBufferClBuffer = smoHooksPtr auto wip_clCollationBufferClBuffer = smoHooksPtr
->ComputeManager_createUseHostPtrBuffer( ->ComputeManager_createUseHostPtrBuffer(
collationBufferPtr, collationBufferSize, CL_MEM_WRITE_ONLY); collationBufferPtr, collationBufferSize, CL_MEM_WRITE_ONLY);
if (!clCollationBufferClBuffer) if (!wip_clCollationBufferClBuffer)
{ {
std::cerr << __func__ << ": failed to create collation buffer" std::cerr << __func__ << ": failed to create collation buffer"
<< std::endl; << std::endl;
goto cleanup; return false;
} }
clAverageIntensityBufferClBuffer = smoHooksPtr auto wip_clAverageIntensityBufferClBuffer = smoHooksPtr
->ComputeManager_createUseHostPtrBuffer( ->ComputeManager_createUseHostPtrBuffer(
averageIntensityBufferPtr, averageIntensityBufferSize, averageIntensityBufferPtr, averageIntensityBufferSize,
CL_MEM_WRITE_ONLY); CL_MEM_WRITE_ONLY);
if (!clAverageIntensityBufferClBuffer) if (!wip_clAverageIntensityBufferClBuffer)
{ {
std::cerr << __func__ << ": failed to create average intensity buffer" std::cerr << __func__ << ": failed to create average intensity buffer"
<< std::endl; << std::endl;
goto cleanup; return false;
} }
// Cache cl_mem handles for the device we're using // Cache cl_mem handles for the device we're using
clAssemblyBuffer = clAssemblyBufferClBuffer cl_mem wip_clAssemblyBuffer = wip_clAssemblyBufferClBuffer
->getAssociatedBufferHandleForDevice(computeDevice); ->getAssociatedBufferHandleForDevice(wip_computeDevice);
clCollationBuffer = clCollationBufferClBuffer cl_mem wip_clCollationBuffer = wip_clCollationBufferClBuffer
->getAssociatedBufferHandleForDevice(computeDevice); ->getAssociatedBufferHandleForDevice(wip_computeDevice);
clAverageIntensityBuffer = clAverageIntensityBufferClBuffer cl_mem wip_clAverageIntensityBuffer = wip_clAverageIntensityBufferClBuffer
->getAssociatedBufferHandleForDevice(computeDevice); ->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::cerr << __func__ << ": failed to get buffer handles for device"
<< std::endl; << std::endl;
goto cleanup; return false;
} }
// Compile and prepare both kernels // Compile and prepare both kernels
if (!compileAndPrepareKernels()) { ClProgramPtr wip_slotCompactorProgram;
goto cleanup; 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); clFlush(computeDevice->commandQueue);
clFinish(computeDevice->commandQueue); clFinish(computeDevice->commandQueue);
shouldAcceptRequests = true; shouldAcceptRequests = true;
return true; return true;
cleanup:
finalize();
return false;
} }
void OpenClCollatingAndMeshingEngine::finalize() void OpenClCollatingAndMeshingEngine::finalize()
@@ -261,29 +278,11 @@ void OpenClCollatingAndMeshingEngine::finalize()
clAverageIntensityBuffer = nullptr; clAverageIntensityBuffer = nullptr;
clAssemblyBuffer = nullptr; clAssemblyBuffer = nullptr;
// Release kernels // Release kernels and programs (handled automatically by unique_ptr destructors)
if (slotCompactorKernel) slotCompactorKernel.reset();
{ collateKernel.reset();
clReleaseKernel(slotCompactorKernel); slotCompactorProgram.reset();
slotCompactorKernel = nullptr; collateProgram.reset();
}
if (collateKernel)
{
clReleaseKernel(collateKernel);
collateKernel = nullptr;
}
// Release programs
if (slotCompactorProgram)
{
clReleaseProgram(slotCompactorProgram);
slotCompactorProgram = nullptr;
}
if (collateProgram)
{
clReleaseProgram(collateProgram);
collateProgram = nullptr;
}
// Release compute device via smo hooks // Release compute device via smo hooks
if (smoHooksPtr && smoHooksPtr->ComputeManager_releaseDevice if (smoHooksPtr && smoHooksPtr->ComputeManager_releaseDevice
@@ -385,7 +384,7 @@ bool OpenClCollatingAndMeshingEngine::startCompactKernel(
unmapAssemblyBuffer(); unmapAssemblyBuffer();
bool success = startKernel( bool success = startKernel(
slotCompactorKernel, slotCompactorKernel.get(),
&currentCompactKernelEvent, &currentCompactKernelEvent,
setupArgs, setupArgs,
validateBuffers, validateBuffers,
@@ -491,7 +490,7 @@ bool OpenClCollatingAndMeshingEngine::startCollateKernel(
size_t globalWorkSize = static_cast<uint32_t>(frameAssemblyDesc->numSlots); size_t globalWorkSize = static_cast<uint32_t>(frameAssemblyDesc->numSlots);
bool success = startKernel( bool success = startKernel(
collateKernel, collateKernel.get(),
&currentCollateKernelEvent, &currentCollateKernelEvent,
setupArgs, setupArgs,
validateBuffers, validateBuffers,
@@ -505,24 +504,28 @@ bool OpenClCollatingAndMeshingEngine::startCollateKernel(
} }
bool OpenClCollatingAndMeshingEngine::compileAndPrepareKernel( bool OpenClCollatingAndMeshingEngine::compileAndPrepareKernel(
const std::shared_ptr<smo::compute::ComputeDevice>& computeDevice,
const char* kernelSource, size_t kernelSourceLen, 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; cl_int err;
// Create program from source // Create program from source
program = clCreateProgramWithSource( cl_program rawProgram = clCreateProgramWithSource(
computeDevice->context, 1, &kernelSource, &kernelSourceLen, &err); computeDevice->context, 1, &kernelSource, &kernelSourceLen, &err);
if (err != CL_SUCCESS || !program) if (err != CL_SUCCESS || !rawProgram)
{ {
std::cerr << __func__ << ": failed to create " << kernelName std::cerr << __func__ << ": failed to create " << kernelName
<< " program: " << err << std::endl; << " program: " << err << std::endl;
return false; return false;
} }
// Wrap in unique_ptr
program = ClProgramPtr(rawProgram);
// Build program // Build program
err = clBuildProgram(program, 1, &computeDevice->device, err = clBuildProgram(program.get(), 1, &computeDevice->device,
nullptr, nullptr, nullptr); nullptr, nullptr, nullptr);
if (err != CL_SUCCESS) if (err != CL_SUCCESS)
@@ -533,14 +536,14 @@ bool OpenClCollatingAndMeshingEngine::compileAndPrepareKernel(
// Print build log if available // Print build log if available
size_t logSize = 0; size_t logSize = 0;
clGetProgramBuildInfo( clGetProgramBuildInfo(
program, computeDevice->device, CL_PROGRAM_BUILD_LOG, program.get(), computeDevice->device, CL_PROGRAM_BUILD_LOG,
0, nullptr, &logSize); 0, nullptr, &logSize);
if (logSize > 0) if (logSize > 0)
{ {
std::vector<char> log(logSize); std::vector<char> log(logSize);
clGetProgramBuildInfo( clGetProgramBuildInfo(
program, computeDevice->device, CL_PROGRAM_BUILD_LOG, program.get(), computeDevice->device, CL_PROGRAM_BUILD_LOG,
logSize, log.data(), nullptr); logSize, log.data(), nullptr);
std::cerr << kernelName << " build log: " << log.data() std::cerr << kernelName << " build log: " << log.data()
<< std::endl; << std::endl;
@@ -550,22 +553,28 @@ bool OpenClCollatingAndMeshingEngine::compileAndPrepareKernel(
} }
// Create kernel // Create kernel
kernel = clCreateKernel(program, kernelName, &err); cl_kernel rawKernel = clCreateKernel(program.get(), kernelName, &err);
if (err != CL_SUCCESS || !kernel) if (err != CL_SUCCESS || !rawKernel)
{ {
std::cerr << __func__ << ": failed to create " << kernelName std::cerr << __func__ << ": failed to create " << kernelName
<< " kernel: " << err << std::endl; << " kernel: " << err << std::endl;
return false; return false;
} }
// Wrap in unique_ptr
kernel = ClKernelPtr(rawKernel);
return true; return true;
} }
bool OpenClCollatingAndMeshingEngine::compileAndPrepareKernels() bool OpenClCollatingAndMeshingEngine::compileAndPrepareKernels(
const std::shared_ptr<smo::compute::ComputeDevice>& computeDevice,
ClProgramPtr& slotCompactorProgram, ClProgramPtr& collateProgram,
ClKernelPtr& slotCompactorKernel, ClKernelPtr& collateKernel)
{ {
// Compile slotCompactor kernel // Compile slotCompactor kernel
if (!compileAndPrepareKernel( if (!compileAndPrepareKernel(
slotCompactorKernelStart, slotCompactorKernelNBytes, computeDevice, slotCompactorKernelStart, slotCompactorKernelNBytes,
"slotCompactor", slotCompactorProgram, slotCompactorKernel)) "slotCompactor", slotCompactorProgram, slotCompactorKernel))
{ {
return false; return false;
@@ -573,7 +582,7 @@ bool OpenClCollatingAndMeshingEngine::compileAndPrepareKernels()
// Compile collateDgrams kernel // Compile collateDgrams kernel
if (!compileAndPrepareKernel( if (!compileAndPrepareKernel(
collateKernelStart, collateKernelNBytes, computeDevice, collateKernelStart, collateKernelNBytes,
"collate", collateProgram, collateKernel)) "collate", collateProgram, collateKernel))
{ {
return false; return false;
@@ -594,7 +603,7 @@ bool OpenClCollatingAndMeshingEngine::setupSlotCompactorsArgs(
// Set kernel arguments for slotCompactor // Set kernel arguments for slotCompactor
cl_int err; cl_int err;
err = clSetKernelArg( err = clSetKernelArg(
slotCompactorKernel, 0, sizeof(cl_mem), &clAssemblyBuffer); slotCompactorKernel.get(), 0, sizeof(cl_mem), &clAssemblyBuffer);
if (err != CL_SUCCESS) if (err != CL_SUCCESS)
{ {
@@ -603,7 +612,8 @@ bool OpenClCollatingAndMeshingEngine::setupSlotCompactorsArgs(
return false; return false;
} }
err = clSetKernelArg(slotCompactorKernel, 1, sizeof(uint32_t), &numSlots); err = clSetKernelArg(
slotCompactorKernel.get(), 1, sizeof(uint32_t), &numSlots);
if (err != CL_SUCCESS) if (err != CL_SUCCESS)
{ {
std::cerr << __func__ << ": failed to set kernel arg 1: " << err std::cerr << __func__ << ": failed to set kernel arg 1: " << err
@@ -611,7 +621,8 @@ bool OpenClCollatingAndMeshingEngine::setupSlotCompactorsArgs(
return false; return false;
} }
err = clSetKernelArg(slotCompactorKernel, 2, sizeof(uint32_t), &slotStride); err = clSetKernelArg(
slotCompactorKernel.get(), 2, sizeof(uint32_t), &slotStride);
if (err != CL_SUCCESS) if (err != CL_SUCCESS)
{ {
std::cerr << __func__ << ": failed to set kernel arg 2: " << err std::cerr << __func__ << ": failed to set kernel arg 2: " << err
@@ -619,7 +630,8 @@ bool OpenClCollatingAndMeshingEngine::setupSlotCompactorsArgs(
return false; return false;
} }
err = clSetKernelArg(slotCompactorKernel, 3, sizeof(uint32_t), &slotSize); err = clSetKernelArg(
slotCompactorKernel.get(), 3, sizeof(uint32_t), &slotSize);
if (err != CL_SUCCESS) if (err != CL_SUCCESS)
{ {
std::cerr << __func__ << ": failed to set kernel arg 3: " << err std::cerr << __func__ << ": failed to set kernel arg 3: " << err
@@ -628,7 +640,7 @@ bool OpenClCollatingAndMeshingEngine::setupSlotCompactorsArgs(
} }
err = clSetKernelArg( err = clSetKernelArg(
slotCompactorKernel, 4, sizeof(uint32_t), &nSucceededUint); slotCompactorKernel.get(), 4, sizeof(uint32_t), &nSucceededUint);
if (err != CL_SUCCESS) if (err != CL_SUCCESS)
{ {
@@ -662,7 +674,8 @@ bool OpenClCollatingAndMeshingEngine::setupCollateDgramsArgs(
// Set kernel arguments for collateDgrams // Set kernel arguments for collateDgrams
cl_int err; cl_int err;
err = clSetKernelArg(collateKernel, 0, sizeof(cl_mem), &clAssemblyBuffer); err = clSetKernelArg(
collateKernel.get(), 0, sizeof(cl_mem), &clAssemblyBuffer);
if (err != CL_SUCCESS) if (err != CL_SUCCESS)
{ {
std::cerr << __func__ << ": failed to set kernel arg 0: " << err std::cerr << __func__ << ": failed to set kernel arg 0: " << err
@@ -670,7 +683,8 @@ bool OpenClCollatingAndMeshingEngine::setupCollateDgramsArgs(
return false; return false;
} }
err = clSetKernelArg(collateKernel, 1, sizeof(cl_mem), &clCollationBuffer); err = clSetKernelArg(
collateKernel.get(), 1, sizeof(cl_mem), &clCollationBuffer);
if (err != CL_SUCCESS) if (err != CL_SUCCESS)
{ {
std::cerr << __func__ << ": failed to set kernel arg 1: " << err std::cerr << __func__ << ": failed to set kernel arg 1: " << err
@@ -685,7 +699,8 @@ bool OpenClCollatingAndMeshingEngine::setupCollateDgramsArgs(
intensityClBuffer = intensityStimFrame->get().clBuffer intensityClBuffer = intensityStimFrame->get().clBuffer
->getAssociatedBufferHandleForDevice(computeDevice); ->getAssociatedBufferHandleForDevice(computeDevice);
} }
err = clSetKernelArg(collateKernel, 2, sizeof(cl_mem), &intensityClBuffer); err = clSetKernelArg(
collateKernel.get(), 2, sizeof(cl_mem), &intensityClBuffer);
if (err != CL_SUCCESS) if (err != CL_SUCCESS)
{ {
std::cerr << __func__ << ": failed to set kernel arg 2: " << err std::cerr << __func__ << ": failed to set kernel arg 2: " << err
@@ -707,7 +722,7 @@ bool OpenClCollatingAndMeshingEngine::setupCollateDgramsArgs(
averageIntensityClBuffer = clAverageIntensityBuffer; averageIntensityClBuffer = clAverageIntensityBuffer;
} }
err = clSetKernelArg( err = clSetKernelArg(
collateKernel, 3, sizeof(cl_mem), &averageIntensityClBuffer); collateKernel.get(), 3, sizeof(cl_mem), &averageIntensityClBuffer);
if (err != CL_SUCCESS) if (err != CL_SUCCESS)
{ {
@@ -716,7 +731,7 @@ bool OpenClCollatingAndMeshingEngine::setupCollateDgramsArgs(
return false; return false;
} }
err = clSetKernelArg(collateKernel, 4, sizeof(uint32_t), &slotStride); err = clSetKernelArg(collateKernel.get(), 4, sizeof(uint32_t), &slotStride);
if (err != CL_SUCCESS) if (err != CL_SUCCESS)
{ {
std::cerr << __func__ << ": failed to set kernel arg 4: " << err std::cerr << __func__ << ": failed to set kernel arg 4: " << err
@@ -724,7 +739,8 @@ bool OpenClCollatingAndMeshingEngine::setupCollateDgramsArgs(
return false; return false;
} }
err = clSetKernelArg(collateKernel, 5, sizeof(uint32_t), &nPointsPerSlot); err = clSetKernelArg(
collateKernel.get(), 5, sizeof(uint32_t), &nPointsPerSlot);
if (err != CL_SUCCESS) if (err != CL_SUCCESS)
{ {
std::cerr << __func__ << ": failed to set kernel arg 5: " << err std::cerr << __func__ << ": failed to set kernel arg 5: " << err
@@ -732,7 +748,8 @@ bool OpenClCollatingAndMeshingEngine::setupCollateDgramsArgs(
return false; return false;
} }
err = clSetKernelArg(collateKernel, 6, sizeof(uint32_t), &nDgramsPerFrame); err = clSetKernelArg(
collateKernel.get(), 6, sizeof(uint32_t), &nDgramsPerFrame);
if (err != CL_SUCCESS) if (err != CL_SUCCESS)
{ {
std::cerr << __func__ << ": failed to set kernel arg 6: " << err 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 // Print all averages above thresholds from average intensity buffer
if (context->ambienceStimFrame.has_value()) if (context->ambienceStimFrame.has_value())
{ {
@@ -5,6 +5,7 @@
#include <cstdint> #include <cstdint>
#include <cstddef> #include <cstddef>
#include <memory> #include <memory>
#include <type_traits>
#include <functional> #include <functional>
#include <optional> #include <optional>
#include <iostream> #include <iostream>
@@ -26,6 +27,25 @@
namespace smo { namespace smo {
namespace stim_buff { 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<cl_program>, ClProgramDeleter>;
using ClKernelPtr = std::unique_ptr<
std::remove_pointer_t<cl_kernel>, ClKernelDeleter>;
class PcloudStimulusProducer; class PcloudStimulusProducer;
class OpenClCollatingAndMeshingEngine class OpenClCollatingAndMeshingEngine
@@ -90,10 +110,10 @@ private:
// OpenCL infrastructure (managed by ComputeManager) // OpenCL infrastructure (managed by ComputeManager)
std::shared_ptr<smo::compute::ComputeDevice> computeDevice; std::shared_ptr<smo::compute::ComputeDevice> computeDevice;
cl_program slotCompactorProgram; ClProgramPtr slotCompactorProgram;
cl_program collateProgram; ClProgramPtr collateProgram;
cl_kernel slotCompactorKernel; ClKernelPtr slotCompactorKernel;
cl_kernel collateKernel; ClKernelPtr collateKernel;
// OpenCL buffers (managed by ComputeManager) // OpenCL buffers (managed by ComputeManager)
std::shared_ptr<smo::compute::ClBuffer> clAssemblyBufferClBuffer; std::shared_ptr<smo::compute::ClBuffer> clAssemblyBufferClBuffer;
@@ -145,9 +165,13 @@ private:
// Private helper methods // Private helper methods
bool compileAndPrepareKernel( bool compileAndPrepareKernel(
const std::shared_ptr<smo::compute::ComputeDevice>& computeDevice,
const char* kernelSource, size_t kernelSourceLen, const char* kernelSource, size_t kernelSourceLen,
const char* kernelName, cl_program& program, cl_kernel& kernel); const char* kernelName, ClProgramPtr& program, ClKernelPtr& kernel);
bool compileAndPrepareKernels(); bool compileAndPrepareKernels(
const std::shared_ptr<smo::compute::ComputeDevice>& computeDevice,
ClProgramPtr& slotCompactorProgram, ClProgramPtr& collateProgram,
ClKernelPtr& slotCompactorKernel, ClKernelPtr& collateKernel);
bool setupSlotCompactorsArgs( bool setupSlotCompactorsArgs(
StagingBuffer& assemblyBuff, uint32_t nSucceeded); StagingBuffer& assemblyBuff, uint32_t nSucceeded);
bool setupCollateDgramsArgs( bool setupCollateDgramsArgs(