[SYCL][PI][L0] Skip modules without kernel inside when creating target kernel on L0 #3614
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Signed-off-by: gejin <[email protected]>
Signed-off-by: gejin <[email protected]>
|
/summary:run |
gmlueck
reviewed
Apr 28, 2021
Signed-off-by: gejin <[email protected]>
Signed-off-by: gejin <[email protected]>
|
/summary:run |
|
/summary:run |
|
Hi, @smaslov-intel |
|
/summary:run |
| @@ -17,16 +17,15 @@ | |||
| #include <cstdarg> | |||
| #include <cstdio> | |||
| #include <cstring> | |||
| #include <level_zero/zes_api.h> | |||
There was a problem hiding this comment.
Why did you move these?
There was a problem hiding this comment.
I ran clang-format to the file before uploading the patch, it should be moved by the format tool.
Thanks very much.
There was a problem hiding this comment.
can you please put them back, perhaps using comments that shut of clang-format
it is weird to have them in the middle of STL includes
There was a problem hiding this comment.
Hi, @smaslov-intel
I just reverted the unnecessary header file change, could you please help approve again?
Thanks very much.
smaslov-intel
suggested changes
May 8, 2021
Signed-off-by: gejin <[email protected]>
smaslov-intel
previously approved these changes
May 12, 2021
Signed-off-by: gejin <[email protected]>
smaslov-intel
approved these changes
May 13, 2021
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
On L0 backend, we iterate all models in a pi_program, for each module, we call zeKernelCreate. If the call fails due to some error other than "ze invalid kernel name", the kernel creation will fail and current program will abort.
However, this logic assume that all modules in the pi_program will have some kernel inside. If we enable SYCL device library "online" link based on piProgramLink, the assumption will be broken. At that time, the pi_program will include some modules from device library without any kernel inside(they are used to resolve undefined symbol in user's image), if we call zeKernelCreate for those module, the return value is "ze invalid argument" instead of "ze invalid kernel name". Suppose under such circumstance, the pi_program includes 2 modules, mod1 for device library without any kernel and mod2 for user's device image with target kernel, we will firstly try to zeKernelCreate for mod1 and the API returns with error "ze invalid argument", then the program will abort but in fact, we can create the target kernel with mod2.
This PR will skip all modules without any kernel inside before trying to call zeKernelCreate for them, this is done by using zeModuelGetKernelNames to query the number of kernel names in module, if kernel name num is 0, we skip it.
Thanks very much.