[SYCL] move more common utils from pi_level_zero #7821
Conversation
| struct _pi_object { | ||
| _pi_object() : RefCount{} {} | ||
|
|
||
| // Level Zero doesn't do the reference counting, so we have to do. |
There was a problem hiding this comment.
Try to get rid of direct mention of Level Zero in "UR"
Also we need to think how this affects backends that do refcnt, e.g. opencl
There was a problem hiding this comment.
Done. Yes, we need to see how useful this _pi_object is for other adapters.
Also, we probably need to do something with the env variables, like SYCL_PI_LEVEL_ZERO_SINGLE_THREAD_MODE. Should we have separate ones for PI and UR?
There was a problem hiding this comment.
I think we need to keep SYCL_PI_LEVEL_ZERO_* ones since customers are already using those, and probably go through some deprecation process for them (suggesting to use new names instead in a warning for some period of time). That said, it looks like we should have 2 options with different names to mean the same thing while we transition.
igchor
force-pushed
the
move_some_common_utils_to_ur
branch
from
86774a7 to
0ee6f42
Compare
Co-authored-by: smaslov-intel <[email protected]>
igchor
force-pushed
the
move_some_common_utils_to_ur
branch
from
0ee6f42 to
988246e
Compare
|
@smaslov-intel what is the general recommendation for what should stay in pi_level_zero.*pp vs what should be transferred to ur/adapters/level_zero/ur_level_zero.*pp? Do we want to end up implementing most of the pi_level_zero interfaces using UR (and just adding interop), or do we want to have a more independent implementation with just a few functions (like *PlatformGet), utils, and data structures shared? As long as we don't introduce any overhead, we can probably start moving the implementation of pi_context, pi_device, pi_queue, etc. to the ur/adapters/level_zero/ur_level_zero.*pp, right? |
Right, we want to move and share as much as possible. |
There was a problem hiding this comment.
Approving with a small comment. I will leave the final say to @smaslov-intel .
| // initialization function provided in Init. All subsequent access to | ||
| // the data just returns the already stored data. | ||
| // | ||
| template <class T> struct ZeCache : private T { |
There was a problem hiding this comment.
I may be wrong, but Ze seems like a leftover from L0-related naming. Should it be renamed to be more general?
There was a problem hiding this comment.
For sure, but renaming it here means re-naming uses in PI L0 plugin too, which might be big.
I'd prefer us to rename things that are completely moved to UR already.
to UR.