[SYCL] Add online linking to Level Zero plugin #2238
Conversation
This commit adds support to the Level Zero plugin for online linking, but the underlying Level Zero driver APIs are still missing. As a short-term workaround, this commit includes mocked versions of those APIs, which work only in the simple case when a single module is "linked" to itself.
| } else { | ||
| assert(Program->State == _pi_program::Object || | ||
| Program->State == _pi_program::Exe || | ||
| Program->State == _pi_program::LinkedExe); |
There was a problem hiding this comment.
Every check for LinkedExe state goes with OR check for Exe state. I think it will simplify the changes if we just have single Exe state to represent both cases
There was a problem hiding this comment.
This is not true. Look at the implementation of ModuleIterator.
There was a problem hiding this comment.
Admitted, but adding anew globally visible state to accommodate internal implementation of iterator is an overkill, in my view. We could check the number/properties of linked objects instead.
There was a problem hiding this comment.
The way I see it, State describes the shape of the data contained by the _pi_program object. There are four possible shapes, so there are four possible states. The LinkedExe state also helps a reader understand the code better. Look at the places where LinkedExe shows up in the comments. Those comments would all have to be changed if we removed this state. Also look at the long comment documenting the LinkedExe state itself. This comment would need to be moved to some less obvious place.
Practically speaking, we could of course replace if (State == LinkedExe) with something like if ((State == Exe) && !LinkedPrograms.empty()). That seems longer and less clear to me, though.
There was a problem hiding this comment.
It's certainly a matter of taste, so I won't insist on changing this now. Please look at the failures reported by the CI. Otherwise changes look good to me.
|
There are few fails in CI, please resolve. |
Programs created via piProgramCreateWithBinary() are always built later via piProgramBuild(). Change piProgramBuild(), so this no longer raises an error. Also fix some other behavior of programs created with piProgramCreateWithBinary(), following the model of the OpenCL spec.
My latest commit should fix these. You might need to approve this PR (again) in order to re-trigger CI testing? |
|
And this commit addresses the CI failure on CUDA. |
Revert "update to L0 loader version v1.18.3 and don't scan _deps with Trivy or CodeQL"
This commit adds support to the Level Zero plugin for online linking,
but the underlying Level Zero driver APIs are still missing. As a
short-term workaround, this commit includes mocked versions of those
APIs, which work only in the simple case when a single module is
"linked" to itself.