[SYCL][L0] Enable global offset support for level_zero. #3593
Conversation
There was a problem hiding this comment.
We need to cache extensions such that the search is not repeated on each kernel launch
There was a problem hiding this comment.
can we further update some local "static bool" value such that the map lookup is performed only once?
There was a problem hiding this comment.
sure, that can be done during the initialization. In that case, defining this generic helper function findDriverExtension() won't be necessary anymore in this PR
There was a problem hiding this comment.
Why do you think we need to give error in this case?
There was a problem hiding this comment.
would you remove this?
There was a problem hiding this comment.
I kept it as it was in Gail's patch. My reason for keeping it is to do an early return and saving the overhead of the next zeDriverGetExtensionProperties call.
There was a problem hiding this comment.
it made sense when it was looking for "global-offset" specifically" but not when it is made a general query
Signed-off-by: rbegam <[email protected]>
Signed-off-by: rbegam <[email protected]>
Signed-off-by: rbegam <[email protected]>
Signed-off-by: rbegam <[email protected]>
Signed-off-by: rbegam <[email protected]>
|
ping @smaslov-intel |
Signed-off-by: rbegam <[email protected]>
|
The pre-commit fail is expected as with this PR the corresponding test (prev marked as XFAIL) is now passing: Unexpectedly Passed Tests (1): The test needs to be enabled which is done in this PR: intel/llvm-test-suite#261 Local testing shows the test passes with this compiler change and vice versa. |
|
Please do not merge the PRs. I have restarted testing to make sure that no regressions introduced. |
Please, let me know when you think we can merge it. I'm also waiting for @romanovvlad to confirm that his comments are addressed. |
|
Now it is OK to merge. Tests have passed |
romanovvlad
left a comment
romanovvlad
left a comment
There was a problem hiding this comment.
Approving to indicate that my minor comment is resolved. Have not reviewed entire patch.
6 participants
Signed-off-by: rbegam [email protected]