aboutsummaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorGil Pitney <gil.pitney@linaro.org>2016-06-29 18:22:54 +0000
committerGil Pitney <gil.pitney@linaro.org>2016-06-29 22:50:45 +0000
commit9461fcfd1434d3cb3c0a1a5de1827d375a1ad5af (patch)
tree829afb9c7fb764a0d551c20a3d793d65dd347594
parentde4b0245f175fd7ce5aab529a190b5520d391755 (diff)
downloadshamrock-9461fcfd1434d3cb3c0a1a5de1827d375a1ad5af.tar.gz
Remove the "resurrection" method in kernel create/delete logic
This removes maintentance of the kernelReleasedList, but maintains the kernelList of the Program object. The logic around resurrecting previously released kernels was not quite correct, and causing issues in kernel object reference counting. This commit also ensures clReleaesKenrel() deletes the kernel, and erases the kernel from the kernelList. This fix causes no regressions in the Khronos test cases, but also fixes a JIT memory leak issue due to undeleted Program objects upon clReleaseProgram(). Signed-off-by: Gil Pitney <gil.pitney@linaro.org>
-rw-r--r--src/api/api_kernel.cpp7
-rw-r--r--src/core/program.cpp62
-rw-r--r--src/core/program.h1
3 files changed, 10 insertions, 60 deletions
diff --git a/src/api/api_kernel.cpp b/src/api/api_kernel.cpp
index 69deddd..ab73398 100644
--- a/src/api/api_kernel.cpp
+++ b/src/api/api_kernel.cpp
@@ -71,7 +71,7 @@ clCreateKernel(cl_program d_program,
if (*errcode_ret != CL_SUCCESS)
{
- delete kernel;
+ // delete kernel; // kernel already deleted by call to createKernelsAndReturnKernel()
return 0;
}
@@ -169,11 +169,8 @@ clReleaseKernel(cl_kernel d_kernel)
{
if (p->kernelList[i]->p_name.compare(kernel->p_name) == 0)
{
- p->kernelReleasedList.push_back(p->kernelList[i]);
p->kernelList.erase(p->kernelList.begin() + i);
- // BUG: TAG
- // For some odd reason when we delete this, we're corrupting then inside of some kernel objects
- //delete kernel;
+ delete kernel;
}
}
}
diff --git a/src/core/program.cpp b/src/core/program.cpp
index 310d012..9e2703e 100644
--- a/src/core/program.cpp
+++ b/src/core/program.cpp
@@ -220,25 +220,10 @@ Kernel *Program::createKernel(const std::string &name, cl_int *errcode_ret)
{
Kernel *rs = NULL;
- for (size_t i=0; i < kernelList.size(); i++)
- {
- if (kernelList[i]->p_name.compare(name) == 0)
- {
- *errcode_ret = CL_SUCCESS;
- return kernelList[i];
- }
- }
- /* Now check the previously released list */
- for (size_t i=0; i < kernelReleasedList.size(); i++)
- {
- if (kernelReleasedList[i]->p_name.compare(name) == 0)
- {
+ for (size_t i=0; i < kernelList.size(); i++) {
+ if (kernelList[i]->p_name.compare(name) == 0) {
*errcode_ret = CL_SUCCESS;
- rs = kernelReleasedList[i];
- kernelReleasedList.erase(kernelReleasedList.begin() + i);
- kernelList.push_back(rs);
-
- return rs;
+ return kernelList[i];
}
}
@@ -296,27 +281,12 @@ Kernel * Program::createKernelsAndReturnKernel(const std::string &name, cl_int *
if (p_device_dependent.size() == 0) return rs;
- for (size_t i=0; i < kernelList.size(); i++)
- {
- if (kernelList[i]->p_name.compare(name) == 0)
- {
+ for (size_t i=0; i < kernelList.size(); i++) {
+ if (kernelList[i]->p_name.compare(name) == 0) {
*errcode_ret = CL_SUCCESS;
- return kernelList[i];
+ return kernelList[i];
}
}
- /* Now check the previously released list */
- for (size_t i=0; i < kernelReleasedList.size(); i++)
- {
- if (kernelReleasedList[i]->p_name.compare(name) == 0)
- {
- *errcode_ret = CL_SUCCESS;
- rs = kernelReleasedList[i];
- kernelReleasedList.erase(kernelReleasedList.begin() + i);
- kernelList.push_back(rs);
-
- return rs;
- }
- }
/*-------------------------------------------------------------------------
* Take the list of kernels for the first device dependent
@@ -351,8 +321,8 @@ Kernel * Program::createKernelsAndReturnKernel(const std::string &name, cl_int *
}
}
- if (!rs && (*errcode_ret == CL_SUCCESS))
- *errcode_ret = CL_INVALID_KERNEL_NAME;
+ if (!rs && (*errcode_ret == CL_SUCCESS))
+ *errcode_ret = CL_INVALID_KERNEL_NAME;
return rs;
}
@@ -368,22 +338,6 @@ std::vector<Kernel *> Program::createKernels(cl_int *errcode_ret)
*------------------------------------------------------------------------*/
if (p_device_dependent.size() == 0) return rs;
- /*
- * Resurrect any released kernels back to the kernel list. This handles the
- * case where clCreateKernelsInProgram() is asking only for a count of kernels in
- * the currently built program. In that case, KernelList.size() must be the actual
- * number of kernels compiled into the program (event if they were previously released).
- */
- //for (size_t i=0; i < kernelReleasedList.size(); i++)
- while(kernelReleasedList.size())
- {
- kern = kernelReleasedList[i];
- kernelReleasedList.erase(kernelReleasedList.begin() + i);
- kernelList.push_back(kern);
- }
-
- if (kernelList.size()) return kernelList;
-
/*-------------------------------------------------------------------------
* Take the list of kernels for the first device dependent
*------------------------------------------------------------------------*/
diff --git a/src/core/program.h b/src/core/program.h
index 7b8e4fc..18bac00 100644
--- a/src/core/program.h
+++ b/src/core/program.h
@@ -285,7 +285,6 @@ class Program : public _cl_program, public Object
std::string source() { return p_source; }
std::vector<Kernel *> kernelList;
- std::vector<Kernel *> kernelReleasedList;
private:
Type p_type;