[Mesa-dev] [RFC PATCH] clover: Return correct CL_EVENT_REFERENCE_COUNT

[Jan Vesely]

On Fri, 2016-12-16 at 13:43 -0800, Francisco Jerez wrote:
> Vedran Miletić <vedran at miletic.net> writes:
> 
> > Current implementation of event handling keeps an extra reference to
> > the hardware event, in addition to the reference returned via the OpenCL
> > API. This additional reference is internal and should not be counted
> > when queried via the clGetEventInfo() function.
> > 
> > Fixes Piglit's cl/api/retain_release-event test.
> > 
> > Signed-off-by: Vedran Miletić <vedran at miletic.net>
> > ---
> >  src/gallium/state_trackers/clover/api/event.cpp | 4 +++-
> >  1 file changed, 3 insertions(+), 1 deletion(-)
> > 
> > diff --git a/src/gallium/state_trackers/clover/api/event.cpp b/src/gallium/state_trackers/clover/api/event.cpp
> > index 5d1a0e5..74bc4d9 100644
> > --- a/src/gallium/state_trackers/clover/api/event.cpp
> > +++ b/src/gallium/state_trackers/clover/api/event.cpp
> > @@ -107,7 +107,9 @@ clGetEventInfo(cl_event d_ev, cl_event_info param,
> >        break;
> >  
> >     case CL_EVENT_REFERENCE_COUNT:
> > -      buf.as_scalar<cl_uint>() = ev.ref_count();
> > +      // Current implementation of event handling keeps an extra reference to
> > +      // the hardware event, which is internal and should not be counted.
> > +      buf.as_scalar<cl_uint>() = ev.ref_count() - 1;
> 
> I don't think this is correct.  There is an internal event reference
> held by the command queue object, but only for as long as the event
> remains in the queue until the next flush.  In other cases the above
> would give you a reference count which is off by one.  That said:
> 
> > The reference count returned should be considered immediately
> > stale. It is unsuitable for general use in applications. This feature
> > is provided for identifying memory leaks.


I found only a generic description that mentions reference count == 1
wrt. events (in Glossary). Even there it says that it's an internal
counter.
The only object that seems to require reference count == 1 is root
device. Contexts, queues, mem, samplers, programs, kernels, events, all
 include the above footnote.
I think the piglit test should be changed to check for non-zero value
instead of 1.

Jan

> >        break;
> >  
> >     default:
> > -- 
> > 2.7.4
> 
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev

-- 
Jan Vesely <jan.vesely at rutgers.edu>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 801 bytes
Desc: This is a digitally signed message part
URL: <https://lists.freedesktop.org/archives/mesa-dev/attachments/20161220/f2da7357/attachment-0001.sig>