-
Notifications
You must be signed in to change notification settings - Fork 762
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[UR] Add handles to opencl adapter #17572
base: sycl
Are you sure you want to change the base?
Conversation
This was from oneapi-src/unified-runtime#1176 . It flagged up some issues in LLVM which needed to be fixed (which they have now been). |
f419feb
to
f66f775
Compare
f66f775
to
fb7aa5e
Compare
fb7aa5e
to
b1daf73
Compare
b1daf73
to
68d37bb
Compare
68d37bb
to
6a7908a
Compare
: CLEvent(Event), Context(Ctx), Queue(Queue) { | ||
RefCount = 1; | ||
urContextRetain(Context); | ||
if (Queue) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
rather than allowing Queue
to be null I think we should query it out of Event
here, otherwise we're introducing a regression when you query EVENT_INFO_QUEUE for an event created from a native handle since the implementation for that query just returns hEvent->Queue
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
actually that doesn't work because the query should return the corresponding ur handle not the native one, not sure what to do about this but it is a regression
size_t refCount = hContext->getReferenceCount(); | ||
// ExtFuncPtrCache is destroyed in an atexit() callback, so it doesn't | ||
// necessarily outlive the adapter (or all the contexts). | ||
if (refCount == 1 && cl_ext::ExtFuncPtrCache) { | ||
cl_ext::ExtFuncPtrCache->clearCache(clContext); | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
could do fewer atomic accesses here
size_t refCount = hContext->getReferenceCount(); | |
// ExtFuncPtrCache is destroyed in an atexit() callback, so it doesn't | |
// necessarily outlive the adapter (or all the contexts). | |
if (refCount == 1 && cl_ext::ExtFuncPtrCache) { | |
cl_ext::ExtFuncPtrCache->clearCache(clContext); | |
} | |
if (hContext->decrementReferenceCount() == 0 ) { | |
// ExtFuncPtrCache is destroyed in an atexit() callback, so it doesn't | |
// necessarily outlive the adapter (or all the contexts). | |
if(cl_ext::ExtFuncPtrCache) { | |
cl_ext::ExtFuncPtrCache->clearCache(clContext); | |
} | |
delete hContext; | |
} |
|
||
std::string SupportedExtensions(ExtStr.c_str()); | ||
if (ExtStr.find("cl_khr_command_buffer") != std::string::npos) { | ||
SupportedExtensions += " ur_exp_command_buffer"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we don't report exp features via extension any more
cl_adapter::cast<cl_event *>(phEvent))); | ||
|
||
NumberOfQueues, &CLQueue, hCommandBuffer->CLCommandBuffer, | ||
numEventsInWaitList, CLWaitEvents.data(), &Event)); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd prefer if we could avoid creating a cl_event
if the user hasn't passed a phEvent
, this is the kind of in-order queue use-case there's an effort to optimize DPC++ for at the moment.
No description provided.