Update spec to clarify event list contents

[RossBrunton]

Updated the spec to state that "If phEventWaitList and phEvent
are not NULL, phEvent must not refer to an element of
the phEventWaitList array."

This is required for OpenCL, and probably makes sense for other
backends as well.

[pbalcer]

Event lists should not contain the output event. This updates all functions that have an event list input and event list output to explicitly state that.

But how would that even be possible? From the user's perspective, it doesn't matter what they set the output event to, since it's going to be overwritten, and the adapter should never read the output event pointer anyway (because it may be uninitialized, and reading uninitialized memory is UB). So even if a user sets the output event to something that exists in the WaitList, it should not matter.

From the adapter's perspective, it cannot modify the input WaitList. So it would not be able to insert the newly allocated output event handle into it.

Am I missing something?

[EwanC]

My thoughts on this are the same as @pbalcer , the input ur_event_handle_t list cannot contain the output ur_event_handle_t list, because it has not been returned yet.

I'm not sure if the intent of this wording to forbid useful like this, however I think this is could be a valid usage idiom to create a linear dependency between commands (although it would leak events without being smarter.)

ur_event_handle_t myEvent;
for (unsigned i=0; i<N; i++) {
  if (i=0) {
     urEnqueueFoo(0, nullptr /*no wait event*/, &myEvent /*set as output*/);
  } else {
     urEnqueueFoo(1, &myEvent /*wait on event*/, &myEvent /*set as output*/);
  }
}

Edit: I see now you need to respect the OpenCL language for "If event_wait_list and event are not NULL, event must not refer to an element of the event_wait_list array." I'm not sure of another solution than propagating this restriction to UR

[pbalcer]

Edit: I see now you need to respect the OpenCL language for "If event_wait_list and event are not NULL, event must not refer to an element of the event_wait_list array." I'm not sure of another solution than propagating this restriction to UR

I'm not a native speaker, so maybe that's on me, but I understand this completely differently than "If phEventWaitList is specified, it must not contain phEvent.".
The OpenCL language states clearly that the pointer to the output event must not be a pointer into the wait list array. The way I understood this PR is that the event wait list can't contain the pointer to the output event.
At the very least the UR language is confusing. 👍 to just copying the OpenCL docs.

[RossBrunton]

@pbalcer I've updated the text to just copy the OpenCL wording, hopefully it's clearer now.

@EwanC The example you've posted is actually what I was trying to stop. I tried doing that pattern elsewhere, and it causes the OpenCL driver on CI to hang.

[pbalcer]

@EwanC The example you've posted is actually what I was trying to stop. I tried doing that pattern elsewhere, and it causes the OpenCL driver on CI to hang.

Maybe we should add a validation layer feature to detect this?

[RossBrunton]

#2196 @pbalcer @EwanC created a ticket for validation here.

[RossBrunton]

Merging without LLVM change - this only touches the spec and comments.