From 89d7fe171d1ca0f88293edd190e51b8fa4c935bb Mon Sep 17 00:00:00 2001 From: Umar Arshad Date: Sat, 30 May 2020 14:42:39 -0400 Subject: [PATCH 1/7] Fix leak of the cl::Buffer object in makeParam. Not leaking cl_mem --- src/backend/opencl/Param.cpp | 5 +++-- src/backend/opencl/Param.hpp | 3 ++- src/backend/opencl/magma/transpose.cpp | 16 +++++++++++----- src/backend/opencl/magma/transpose_inplace.cpp | 13 +++++++++---- 4 files changed, 25 insertions(+), 12 deletions(-) diff --git a/src/backend/opencl/Param.cpp b/src/backend/opencl/Param.cpp index 34a01f4a5d..25358310ae 100644 --- a/src/backend/opencl/Param.cpp +++ b/src/backend/opencl/Param.cpp @@ -16,9 +16,10 @@ namespace opencl { Param::Param() : data(nullptr), info{{0, 0, 0, 0}, {0, 0, 0, 0}, 0} {} Param::Param(cl::Buffer *data_, KParam info_) : data(data_), info(info_) {} -Param makeParam(cl_mem mem, int off, const int dims[4], const int strides[4]) { +Param makeParam(cl::Buffer &mem, int off, const int dims[4], + const int strides[4]) { Param out; - out.data = new cl::Buffer(mem); + out.data = &mem; out.info.offset = off; for (int i = 0; i < 4; i++) { out.info.dims[i] = dims[i]; diff --git a/src/backend/opencl/Param.hpp b/src/backend/opencl/Param.hpp index 85f010f2d2..6cf63f356b 100644 --- a/src/backend/opencl/Param.hpp +++ b/src/backend/opencl/Param.hpp @@ -29,5 +29,6 @@ struct Param { }; // AF_DEPRECATED("Use Array") -Param makeParam(cl_mem mem, int off, const int dims[4], const int strides[4]); +Param makeParam(cl::Buffer& mem, int off, const int dims[4], + const int strides[4]); } // namespace opencl diff --git a/src/backend/opencl/magma/transpose.cpp b/src/backend/opencl/magma/transpose.cpp index 7ccb71eb4a..e9ff2243ca 100644 --- a/src/backend/opencl/magma/transpose.cpp +++ b/src/backend/opencl/magma/transpose.cpp @@ -54,6 +54,11 @@ #include "kernel/transpose.hpp" #include "magma_data.h" +using cl::Buffer; +using cl::CommandQueue; +using opencl::makeParam; +using opencl::kernel::transpose; + template void magmablas_transpose(magma_int_t m, magma_int_t n, cl_mem dA, size_t dA_offset, magma_int_t ldda, cl_mem dAT, @@ -83,12 +88,13 @@ void magmablas_transpose(magma_int_t m, magma_int_t n, cl_mem dA, int istrides[] = {1, ldda, ldda * n, ldda * n}; int ostrides[] = {1, lddat, lddat * m, lddat * m}; - using namespace opencl; + Buffer dATBuf(dAT, true); + Buffer dABuf(dA, true); - cl::CommandQueue q(queue, true); - kernel::transpose(makeParam(dAT, dAT_offset, odims, ostrides), - makeParam(dA, dA_offset, idims, istrides), q, false, - m % 32 == 0 && n % 32 == 0); + CommandQueue q(queue, true); + transpose(makeParam(dATBuf, dAT_offset, odims, ostrides), + makeParam(dABuf, dA_offset, idims, istrides), q, false, + m % 32 == 0 && n % 32 == 0); } #define INSTANTIATE(T) \ diff --git a/src/backend/opencl/magma/transpose_inplace.cpp b/src/backend/opencl/magma/transpose_inplace.cpp index 6f649f55bb..21770f98be 100644 --- a/src/backend/opencl/magma/transpose_inplace.cpp +++ b/src/backend/opencl/magma/transpose_inplace.cpp @@ -54,6 +54,11 @@ #include "kernel/transpose_inplace.hpp" #include "magma_data.h" +using cl::Buffer; +using cl::CommandQueue; +using opencl::makeParam; +using opencl::kernel::transpose_inplace; + template void magmablas_transpose_inplace(magma_int_t n, cl_mem dA, size_t dA_offset, magma_int_t ldda, magma_queue_t queue) { @@ -74,11 +79,11 @@ void magmablas_transpose_inplace(magma_int_t n, cl_mem dA, size_t dA_offset, int dims[] = {n, n, 1, 1}; int strides[] = {1, ldda, ldda * n, ldda * n}; - using namespace opencl; + Buffer dABuf(dA, true); - cl::CommandQueue q(queue, true); - kernel::transpose_inplace(makeParam(dA, dA_offset, dims, strides), q, - false, n % 32 == 0); + CommandQueue q(queue, true); + transpose_inplace(makeParam(dABuf, dA_offset, dims, strides), q, false, + n % 32 == 0); } #define INSTANTIATE(T) \ From 1cd84784c98a1ec0a0fda6d8a0fa51bc72618f5a Mon Sep 17 00:00:00 2001 From: Umar Arshad Date: Sat, 30 May 2020 15:15:22 -0400 Subject: [PATCH 2/7] Refactor Modules and Kernels. Fix leak in getKernel Refactor Module and fix a leak of the cl::Kernel objects. These objects should be around for a while so the accumulated leak wasn't significant in most applications. --- CMakeModules/LSANSuppression.txt | 3 +- src/backend/common/ModuleInterface.hpp | 11 +++- src/backend/common/kernel_cache.cpp | 12 ++-- src/backend/cuda/Module.hpp | 3 + src/backend/opencl/Kernel.hpp | 14 ++--- src/backend/opencl/Module.hpp | 17 ++++-- src/backend/opencl/compile_module.cpp | 82 +++++++++++++------------- src/backend/opencl/jit.cpp | 26 ++++---- 8 files changed, 94 insertions(+), 74 deletions(-) diff --git a/CMakeModules/LSANSuppression.txt b/CMakeModules/LSANSuppression.txt index dca058df0f..0026fbc27d 100644 --- a/CMakeModules/LSANSuppression.txt +++ b/CMakeModules/LSANSuppression.txt @@ -1,8 +1,7 @@ # This is a known leak. -leak:getKernel -#leak:libOpenCL leak:libnvidia-ptxjitcompile leak:tbb::internal::task_stream +leak:libnvidia-opencl.so # Allocated by Intel's OpenMP implementation during inverse_dense_cpu # This is not something we can control in ArrayFire diff --git a/src/backend/common/ModuleInterface.hpp b/src/backend/common/ModuleInterface.hpp index 052a661916..167c3b2304 100644 --- a/src/backend/common/ModuleInterface.hpp +++ b/src/backend/common/ModuleInterface.hpp @@ -18,6 +18,12 @@ class ModuleInterface { ModuleType mModuleHandle; public: + /// \brief Creates an uninitialized Module + ModuleInterface() = default; + + /// \brief Creates a module given a backend specific ModuleType + /// + /// \param[in] mod The backend specific module ModuleInterface(ModuleType mod) : mModuleHandle(mod) {} /// \brief Set module @@ -28,10 +34,13 @@ class ModuleInterface { /// \brief Get module /// /// \returns handle to backend specific module - inline ModuleType get() const { return mModuleHandle; } + inline const ModuleType& get() const { return mModuleHandle; } /// \brief Unload module virtual void unload() = 0; + + /// \brief Returns true if the module mModuleHandle is initialized + virtual operator bool() const = 0; }; } // namespace common diff --git a/src/backend/common/kernel_cache.cpp b/src/backend/common/kernel_cache.cpp index 0c879070a1..79c6e1c3eb 100644 --- a/src/backend/common/kernel_cache.cpp +++ b/src/backend/common/kernel_cache.cpp @@ -42,7 +42,8 @@ shared_timed_mutex& getCacheMutex(const int device) { } ModuleMap& getCache(const int device) { - static ModuleMap caches[detail::DeviceManager::MAX_DEVICES]; + static ModuleMap* caches = + new ModuleMap[detail::DeviceManager::MAX_DEVICES]; return caches[device]; } @@ -51,7 +52,7 @@ Module findModule(const int device, const string& key) { auto& cache = getCache(device); auto iter = cache.find(key); if (iter != cache.end()) { return iter->second; } - return Module{nullptr}; + return Module{}; } Kernel getKernel(const string& kernelName, const vector& sources, @@ -89,9 +90,9 @@ Kernel getKernel(const string& kernelName, const vector& sources, const int device = detail::getActiveDeviceId(); Module currModule = findModule(device, moduleKey); - if (currModule.get() == nullptr) { + if (!currModule) { currModule = loadModuleFromDisk(device, moduleKey, sourceIsJIT); - if (currModule.get() == nullptr) { + if (!currModule) { currModule = compileModule(moduleKey, sources, options, {tInstance}, sourceIsJIT); } @@ -102,7 +103,8 @@ Kernel getKernel(const string& kernelName, const vector& sources, if (iter == cache.end()) { // If not found, this thread is the first one to compile this // kernel. Keep the generated module. - getCache(device).emplace(moduleKey, currModule); + Module mod = currModule; + getCache(device).emplace(moduleKey, mod); } else { currModule.unload(); // dump the current threads extra compilation currModule = iter->second; diff --git a/src/backend/cuda/Module.hpp b/src/backend/cuda/Module.hpp index d910d1f90c..ceefd2f94e 100644 --- a/src/backend/cuda/Module.hpp +++ b/src/backend/cuda/Module.hpp @@ -28,10 +28,13 @@ class Module : public common::ModuleInterface { using ModuleType = CUmodule; using BaseClass = common::ModuleInterface; + Module() = default; Module(ModuleType mod) : BaseClass(mod) { mInstanceMangledNames.reserve(1); } + operator bool() const final { return get(); } + void unload() final { CU_CHECK(cuModuleUnload(get())); set(nullptr); diff --git a/src/backend/opencl/Kernel.hpp b/src/backend/opencl/Kernel.hpp index 3284fea367..9953a4d956 100644 --- a/src/backend/opencl/Kernel.hpp +++ b/src/backend/opencl/Kernel.hpp @@ -18,24 +18,24 @@ namespace opencl { struct Enqueuer { template - void operator()(void* ker, const cl::EnqueueArgs& qArgs, Args... args) { - auto launchOp = - cl::KernelFunctor(*static_cast(ker)); + void operator()(cl::Kernel ker, const cl::EnqueueArgs& qArgs, + Args... args) { + auto launchOp = cl::KernelFunctor(ker); launchOp(qArgs, std::forward(args)...); } }; class Kernel - : public common::KernelInterface { public: - using ModuleType = cl::Program*; - using KernelType = cl::Kernel*; + using ModuleType = const cl::Program*; + using KernelType = cl::Kernel; using DevPtrType = cl::Buffer*; using BaseClass = common::KernelInterface; - Kernel() : BaseClass(nullptr, nullptr) {} + Kernel() : BaseClass(nullptr, cl::Kernel{nullptr, false}) {} Kernel(ModuleType mod, KernelType ker) : BaseClass(mod, ker) {} // clang-format off diff --git a/src/backend/opencl/Module.hpp b/src/backend/opencl/Module.hpp index c0bafeadec..c918797699 100644 --- a/src/backend/opencl/Module.hpp +++ b/src/backend/opencl/Module.hpp @@ -16,17 +16,22 @@ namespace opencl { /// OpenCL backend wrapper for cl::Program object -class Module : public common::ModuleInterface { +class Module : public common::ModuleInterface { public: - using ModuleType = cl::Program*; + using ModuleType = cl::Program; using BaseClass = common::ModuleInterface; + /// \brief Create an uninitialized Module + Module() = default; + + /// \brief Create a module given a cl::Program type Module(ModuleType mod) : BaseClass(mod) {} - void unload() final { - delete get(); - set(nullptr); - } + /// \brief Unload module + operator bool() const final { return get()(); } + + /// Unload the module + void unload() final { set(cl::Program()); } }; } // namespace opencl diff --git a/src/backend/opencl/compile_module.cpp b/src/backend/opencl/compile_module.cpp index 69f4414eb6..fab31558b0 100644 --- a/src/backend/opencl/compile_module.cpp +++ b/src/backend/opencl/compile_module.cpp @@ -25,39 +25,48 @@ #include #include -using detail::Kernel; -using detail::Module; - +using cl::Error; +using cl::Program; +using common::loggerFactory; +using opencl::getActiveDeviceId; +using opencl::getDevice; +using opencl::Kernel; +using opencl::Module; +using spdlog::logger; + +using std::begin; +using std::end; using std::ostringstream; +using std::shared_ptr; using std::string; using std::vector; using std::chrono::duration_cast; using std::chrono::high_resolution_clock; using std::chrono::milliseconds; -spdlog::logger *getLogger() { - static std::shared_ptr logger(common::loggerFactory("jit")); +logger *getLogger() { + static shared_ptr logger(loggerFactory("jit")); return logger.get(); } -#define SHOW_DEBUG_BUILD_INFO(PROG) \ - do { \ - cl_uint numDevices = PROG->getInfo(); \ - for (unsigned int i = 0; i < numDevices; ++i) { \ - printf("%s\n", PROG->getBuildInfo( \ - PROG->getInfo()[i]) \ - .c_str()); \ - printf("%s\n", PROG->getBuildInfo( \ - PROG->getInfo()[i]) \ - .c_str()); \ - } \ +#define SHOW_DEBUG_BUILD_INFO(PROG) \ + do { \ + cl_uint numDevices = PROG.getInfo(); \ + for (unsigned int i = 0; i < numDevices; ++i) { \ + printf("%s\n", PROG.getBuildInfo( \ + PROG.getInfo()[i]) \ + .c_str()); \ + printf("%s\n", PROG.getBuildInfo( \ + PROG.getInfo()[i]) \ + .c_str()); \ + } \ } while (0) #if defined(NDEBUG) #define SHOW_BUILD_INFO(PROG) \ do { \ - std::string info = getEnvVar("AF_OPENCL_SHOW_BUILD_INFO"); \ + string info = getEnvVar("AF_OPENCL_SHOW_BUILD_INFO"); \ if (!info.empty() && info != "0") { SHOW_DEBUG_BUILD_INFO(PROG); } \ } while (0) @@ -67,7 +76,7 @@ spdlog::logger *getLogger() { namespace opencl { -const static std::string DEFAULT_MACROS_STR( +const static string DEFAULT_MACROS_STR( "\n\ #ifdef USE_DOUBLE\n\ #pragma OPENCL EXTENSION cl_khr_fp64 : enable\n\ @@ -82,36 +91,32 @@ const static std::string DEFAULT_MACROS_STR( #endif\n \ "); -cl::Program *buildProgram(const std::vector &kernelSources, - const std::vector &compileOpts) { - using std::begin; - using std::end; - - cl::Program *retVal = nullptr; +Program buildProgram(const vector &kernelSources, + const vector &compileOpts) { + Program retVal; try { - static const std::string defaults = - std::string(" -D dim_t=") + - std::string(dtype_traits::getName()); + static const string defaults = + string(" -D dim_t=") + string(dtype_traits::getName()); auto device = getDevice(); - const std::string cl_std = - std::string(" -cl-std=CL") + + const string cl_std = + string(" -cl-std=CL") + device.getInfo().substr(9, 3); - cl::Program::Sources sources; + Program::Sources sources; sources.emplace_back(DEFAULT_MACROS_STR); sources.emplace_back(KParam_hpp, KParam_hpp_len); sources.insert(end(sources), begin(kernelSources), end(kernelSources)); - retVal = new cl::Program(getContext(), sources); + retVal = Program(getContext(), sources); ostringstream options; for (auto &opt : compileOpts) { options << opt; } - retVal->build({device}, (cl_std + defaults + options.str()).c_str()); - } catch (...) { - if (retVal) { SHOW_BUILD_INFO(retVal); } + retVal.build({device}, (cl_std + defaults + options.str()).c_str()); + } catch (Error &err) { + if (err.err() == CL_BUILD_ERROR) { SHOW_BUILD_INFO(retVal); } throw; } return retVal; @@ -124,14 +129,11 @@ namespace common { Module compileModule(const string &moduleKey, const vector &sources, const vector &options, const vector &kInstances, const bool isJIT) { - using opencl::getActiveDeviceId; - using opencl::getDevice; - UNUSED(kInstances); UNUSED(isJIT); auto compileBegin = high_resolution_clock::now(); - auto program = detail::buildProgram(sources, options); + auto program = opencl::buildProgram(sources, options); auto compileEnd = high_resolution_clock::now(); AF_TRACE("{{{:<30} : {{ compile:{:>5} ms, {{ {} }}, {} }}}}", moduleKey, @@ -147,13 +149,13 @@ Module loadModuleFromDisk(const int device, const string &moduleKey, UNUSED(device); UNUSED(moduleKey); UNUSED(isJIT); - return {nullptr}; + return {}; } Kernel getKernel(const Module &mod, const string &nameExpr, const bool sourceWasJIT) { UNUSED(sourceWasJIT); - return {mod.get(), new cl::Kernel(*mod.get(), nameExpr.c_str())}; + return {&mod.get(), cl::Kernel(mod.get(), nameExpr.c_str())}; } } // namespace common diff --git a/src/backend/opencl/jit.cpp b/src/backend/opencl/jit.cpp index ac28c3f50f..b49521cffd 100644 --- a/src/backend/opencl/jit.cpp +++ b/src/backend/opencl/jit.cpp @@ -137,10 +137,10 @@ string getKernelString(const string &funcName, const vector &full_nodes, return kerStream.str(); } -cl::Kernel *getKernel(const vector &output_nodes, - const vector &output_ids, - const vector &full_nodes, - const vector &full_ids, const bool is_linear) { +cl::Kernel getKernel(const vector &output_nodes, + const vector &output_ids, + const vector &full_nodes, + const vector &full_ids, const bool is_linear) { const string funcName = getFuncName(output_nodes, full_nodes, full_ids, is_linear); const string moduleKey = std::to_string(deterministicHash(funcName)); @@ -150,7 +150,7 @@ cl::Kernel *getKernel(const vector &output_nodes, // with a way to save jit kernels to disk only once auto entry = common::findModule(getActiveDeviceId(), moduleKey); - if (entry.get() == nullptr) { + if (!entry) { static const string jit(jit_cl, jit_cl_len); string jitKer = getKernelString(funcName, full_nodes, full_ids, @@ -252,25 +252,25 @@ void evalNodes(vector &outputs, const vector &output_nodes) { for (const auto &node : full_nodes) { nargs = node->setArgs(nargs, is_linear, [&](int id, const void *ptr, size_t arg_size) { - ker->setArg(id, arg_size, ptr); + ker.setArg(id, arg_size, ptr); }); } // Set output parameters - for (auto output : outputs) { - ker->setArg(nargs, *(output.data)); + for (auto &output : outputs) { + ker.setArg(nargs, *(output.data)); ++nargs; } // Set dimensions // All outputs are asserted to be of same size // Just use the size from the first output - ker->setArg(nargs + 0, out_info); - ker->setArg(nargs + 1, groups_0); - ker->setArg(nargs + 2, groups_1); - ker->setArg(nargs + 3, num_odims); + ker.setArg(nargs + 0, out_info); + ker.setArg(nargs + 1, groups_0); + ker.setArg(nargs + 2, groups_1); + ker.setArg(nargs + 3, num_odims); - getQueue().enqueueNDRangeKernel(*ker, NullRange, global, local); + getQueue().enqueueNDRangeKernel(ker, NullRange, global, local); // Reset the thread local vectors nodes.clear(); From b285f3c0ee02b8d158c240e19b9595aba446045f Mon Sep 17 00:00:00 2001 From: Umar Arshad Date: Sat, 30 May 2020 15:24:45 -0400 Subject: [PATCH 3/7] Fix mismatch new/delete calls in clfft --- CMakeModules/LSANSuppression.txt | 1 + src/backend/opencl/Array.cpp | 4 +--- src/backend/opencl/clfft.cpp | 3 +-- test/meanvar.cpp | 3 ++- 4 files changed, 5 insertions(+), 6 deletions(-) diff --git a/CMakeModules/LSANSuppression.txt b/CMakeModules/LSANSuppression.txt index 0026fbc27d..43ac584d10 100644 --- a/CMakeModules/LSANSuppression.txt +++ b/CMakeModules/LSANSuppression.txt @@ -2,6 +2,7 @@ leak:libnvidia-ptxjitcompile leak:tbb::internal::task_stream leak:libnvidia-opencl.so +leak:FFTRepo::FFTRepoKey::privatizeData # Allocated by Intel's OpenMP implementation during inverse_dense_cpu # This is not something we can control in ArrayFire diff --git a/src/backend/opencl/Array.cpp b/src/backend/opencl/Array.cpp index f7bd205aa2..c47fc56ee0 100644 --- a/src/backend/opencl/Array.cpp +++ b/src/backend/opencl/Array.cpp @@ -246,9 +246,7 @@ void evalMultiple(vector *> arrays) { info.strides()[3]}, 0}; - Param res = {array->data.get(), kInfo}; - - outputs.push_back(res); + outputs.emplace_back(array->data.get(), kInfo); output_arrays.push_back(array); nodes.push_back(array->node.get()); } diff --git a/src/backend/opencl/clfft.cpp b/src/backend/opencl/clfft.cpp index 1ae27c85cf..21ef1f37d7 100644 --- a/src/backend/opencl/clfft.cpp +++ b/src/backend/opencl/clfft.cpp @@ -169,8 +169,7 @@ SharedPlan findPlan(clfftLayout iLayout, clfftLayout oLayout, clfftDim rank, // thrown. This is related to // https://github.com/arrayfire/arrayfire/pull/1899 CLFFT_CHECK(clfftDestroyPlan(p)); - // NOLINTNEXTLINE(hicpp-no-malloc) - free(p); + delete p; #endif }); // push the plan into plan cache diff --git a/test/meanvar.cpp b/test/meanvar.cpp index f7519aed47..059f694842 100644 --- a/test/meanvar.cpp +++ b/test/meanvar.cpp @@ -26,6 +26,7 @@ using std::move; using std::string; using std::vector; +af_err init_err = af_init(); template struct elseType { typedef typename cond_type::value || @@ -91,7 +92,7 @@ struct meanvar_test { ~meanvar_test() { #ifndef _WIN32 - af_release_array(in_); + if (in_) af_release_array(in_); if (weights_) { af_release_array(weights_); weights_ = 0; From f3fccdb62da6722c389c19fabed8236b01ee6ee2 Mon Sep 17 00:00:00 2001 From: Umar Arshad Date: Sat, 30 May 2020 15:46:29 -0400 Subject: [PATCH 4/7] Fix leak in susan --- src/backend/opencl/kernel/susan.hpp | 6 ++--- src/backend/opencl/susan.cpp | 34 ++++++++++++----------------- 2 files changed, 17 insertions(+), 23 deletions(-) diff --git a/src/backend/opencl/kernel/susan.hpp b/src/backend/opencl/kernel/susan.hpp index 09f1c1c6d5..f22b8607e1 100644 --- a/src/backend/opencl/kernel/susan.hpp +++ b/src/backend/opencl/kernel/susan.hpp @@ -15,6 +15,7 @@ #include #include #include +#include #include #include @@ -81,8 +82,8 @@ unsigned nonMaximal(cl::Buffer* x_out, cl::Buffer* y_out, cl::Buffer* resp_out, auto nonMax = common::getKernel("non_maximal", {susanSrc()}, targs, compileOpts); - unsigned corners_found = 0; - cl::Buffer* d_corners_found = bufferAlloc(sizeof(unsigned)); + unsigned corners_found = 0; + auto d_corners_found = memAlloc(1); getQueue().enqueueWriteBuffer(*d_corners_found, CL_FALSE, 0, sizeof(unsigned), &corners_found); @@ -95,7 +96,6 @@ unsigned nonMaximal(cl::Buffer* x_out, cl::Buffer* y_out, cl::Buffer* resp_out, max_corners); getQueue().enqueueReadBuffer(*d_corners_found, CL_TRUE, 0, sizeof(unsigned), &corners_found); - bufferFree(d_corners_found); return corners_found; } } // namespace kernel diff --git a/src/backend/opencl/susan.cpp b/src/backend/opencl/susan.cpp index 6b5cc5e1f3..35f22a953b 100644 --- a/src/backend/opencl/susan.cpp +++ b/src/backend/opencl/susan.cpp @@ -15,6 +15,7 @@ #include using af::features; +using std::vector; namespace opencl { @@ -26,38 +27,31 @@ unsigned susan(Array &x_out, Array &y_out, Array &resp_out, dim4 idims = in.dims(); const unsigned corner_lim = in.elements() * feature_ratio; - cl::Buffer *x_corners = bufferAlloc(corner_lim * sizeof(float)); - cl::Buffer *y_corners = bufferAlloc(corner_lim * sizeof(float)); - cl::Buffer *resp_corners = bufferAlloc(corner_lim * sizeof(float)); + Array x_corners = createEmptyArray({corner_lim}); + Array y_corners = createEmptyArray({corner_lim}); + Array resp_corners = createEmptyArray({corner_lim}); - cl::Buffer *resp = bufferAlloc(in.elements() * sizeof(float)); + auto resp = memAlloc(in.elements()); - kernel::susan(resp, in.get(), in.getOffset(), idims[0], idims[1], + kernel::susan(resp.get(), in.get(), in.getOffset(), idims[0], idims[1], diff_thr, geom_thr, edge, radius); - unsigned corners_found = - kernel::nonMaximal(x_corners, y_corners, resp_corners, idims[0], - idims[1], resp, edge, corner_lim); - bufferFree(resp); + unsigned corners_found = kernel::nonMaximal( + x_corners.get(), y_corners.get(), resp_corners.get(), idims[0], + idims[1], resp.get(), edge, corner_lim); const unsigned corners_out = std::min(corners_found, corner_lim); if (corners_out == 0) { - bufferFree(x_corners); - bufferFree(y_corners); - bufferFree(resp_corners); x_out = createEmptyArray(dim4()); y_out = createEmptyArray(dim4()); resp_out = createEmptyArray(dim4()); - return 0; } else { - x_out = createDeviceDataArray(dim4(corners_out), - (void *)((*x_corners)())); - y_out = createDeviceDataArray(dim4(corners_out), - (void *)((*y_corners)())); - resp_out = createDeviceDataArray(dim4(corners_out), - (void *)((*resp_corners)())); - return corners_out; + vector idx{{0., static_cast(corners_out - 1.0), 1.}}; + x_out = createSubArray(x_corners, idx); + y_out = createSubArray(y_corners, idx); + resp_out = createSubArray(resp_corners, idx); } + return corners_out; } #define INSTANTIATE(T) \ From bf461efe761782a6f4c1a1ce5ded89923ca47e6d Mon Sep 17 00:00:00 2001 From: Umar Arshad Date: Sat, 30 May 2020 15:47:21 -0400 Subject: [PATCH 5/7] Fix leak in sparseArith --- src/backend/opencl/kernel/sparse_arith.hpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/backend/opencl/kernel/sparse_arith.hpp b/src/backend/opencl/kernel/sparse_arith.hpp index 8e42e0b96f..78331ed587 100644 --- a/src/backend/opencl/kernel/sparse_arith.hpp +++ b/src/backend/opencl/kernel/sparse_arith.hpp @@ -156,8 +156,8 @@ static void csrCalcOutNNZ(Param outRowIdx, unsigned &nnzC, const uint M, cl::NDRange local(256, 1); cl::NDRange global(divup(M, local[0]) * local[0], 1, 1); - nnzC = 0; - cl::Buffer *out = bufferAlloc(sizeof(unsigned)); + nnzC = 0; + auto out = memAlloc(1); getQueue().enqueueWriteBuffer(*out, CL_TRUE, 0, sizeof(unsigned), &nnzC); calcNNZ(cl::EnqueueArgs(getQueue(), global, local), *out, *outRowIdx.data, From 9fa81a1f6c07a178587391086f6d271dbca62bba Mon Sep 17 00:00:00 2001 From: Umar Arshad Date: Sat, 30 May 2020 15:47:53 -0400 Subject: [PATCH 6/7] Fix leak in OpenCL ireduce --- src/backend/opencl/kernel/ireduce.hpp | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/src/backend/opencl/kernel/ireduce.hpp b/src/backend/opencl/kernel/ireduce.hpp index 3fb8a1633b..39e6497d4e 100644 --- a/src/backend/opencl/kernel/ireduce.hpp +++ b/src/backend/opencl/kernel/ireduce.hpp @@ -183,18 +183,20 @@ void ireduceFirst(Param out, cl::Buffer *oidx, Param in, Param rlen) { template void ireduce(Param out, cl::Buffer *oidx, Param in, int dim, Param rlen) { + cl::Buffer buf; if (rlen.info.dims[0] * rlen.info.dims[1] * rlen.info.dims[2] * rlen.info.dims[3] == 0) { // empty opencl::Param() does not have nullptr by default // set to nullptr explicitly here for consequent kernel calls // through cl::Buffer's constructor - rlen.data = new cl::Buffer(); + rlen.data = &buf; + } + if (dim == 0) { + ireduceFirst(out, oidx, in, rlen); + } else { + ireduceDim(out, oidx, in, dim, rlen); } - if (dim == 0) - return ireduceFirst(out, oidx, in, rlen); - else - return ireduceDim(out, oidx, in, dim, rlen); } #if defined(__GNUC__) || defined(__GNUG__) From f11c261e3425ccc2b144dbe26510876e6cc570aa Mon Sep 17 00:00:00 2001 From: Umar Arshad Date: Sat, 30 May 2020 15:48:52 -0400 Subject: [PATCH 7/7] Fix leak in OpenCL Indexing --- src/backend/opencl/index.cpp | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/src/backend/opencl/index.cpp b/src/backend/opencl/index.cpp index 5433401387..a5d00b8373 100644 --- a/src/backend/opencl/index.cpp +++ b/src/backend/opencl/index.cpp @@ -45,6 +45,7 @@ Array index(const Array& in, const af_index_t idxrs[]) { cl::Buffer* bPtrs[4]; + auto buf = cl::Buffer(); std::vector> idxArrs(4, createEmptyArray(dim4())); // look through indexs to read af_array indexs for (dim_t x = 0; x < 4; ++x) { @@ -56,7 +57,7 @@ Array index(const Array& in, const af_index_t idxrs[]) { oDims[x] = idxArrs[x].elements(); } else { // alloc an 1-element buffer to avoid OpenCL from failing - bPtrs[x] = bufferAlloc(sizeof(uint)); + bPtrs[x] = &buf; } } @@ -65,10 +66,6 @@ Array index(const Array& in, const af_index_t idxrs[]) { kernel::index(out, in, p, bPtrs); - for (dim_t x = 0; x < 4; ++x) { - if (p.isSeq[x]) { bufferFree(bPtrs[x]); } - } - return out; }