-
Couldn't load subscription status.
- Fork 253
Remove indirection from ocloc_lib and ocloc names #456
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Conversation
zboszor
commented
Sep 18, 2021
|
|
||
| function(level_zero_gen_kernels target platform_name suffix options) | ||
|
|
||
| if(NOT DEFINED cloc_cmd_prefix) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
could you pass -Dcloc_cmd_prefix=ocloc to cmake command in your environment instead of removing this code?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The problem is that with two or more levels of variable indirection, cmake doesn't apply the CMAKE_CROSSCOMPILING_EMULATOR wrapper command and this is the case with cloc_cmd_prefix.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For these add_custom_command there is only one level of indirection - usage of ${cloc_cmd_prefix}. The problem I see with using CMAKE_CROSSCOMPILING_EMULATOR is that cmake adds CMAKE_CROSSCOMPILING_EMULATOR before command and in our default scenario there will be something like ${CMAKE_CROSSCOMPILING_EMULATOR} LD_LIBRARY_PATH=... ocloc ... which will cause issue as environment variable cannot be defined in the middle of cmdline. Therefore I hope that if you define cloc_cmd_prefix as ocloc then you will get ${CMAKE_CROSSCOMPILING_EMULATOR} ocloc ... which looks like a valid cmdline. Please double check that.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@zboszor have you checked if -Dcloc_cmd_prefix=ocloc helps for the issue?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, I checked. It is this way in meta-intel and it's good if ocloc is built for running on the crosscompiling host.
But it is broken if coupled with -DCMAKE_CROSSCOMPILING_EMULATOR=...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
could you share build logs? what exactly is broken?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
could you share build logs? what exactly is broken?
I don't have the build logs.
The issue is that the ocloc command is not prefixed with the crosscompiling wrapper.
If ocloc is cross-built for the target, it can't run properly using the host libraries.
E.g. the Yocto cross-build is not multilib but the host is, the runtime linker path is not good to run the binary without the wrapper that executes the binary in qemu, etc.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@zboszor could you verify if it is still needed? cross-build with qemu works fine for us
| # | ||
|
|
||
| project(${OCLOC_NAME}_lib) | ||
| project(ocloc_lib) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why is this change needed? I see what can be incorrect with our current implementation - project is defined before OCLOC_NAME is defined which evaluates to project(_lib) . Simply moving set(OCLOC_NAME "ocloc") to a line before project(${OCLOC_NAME}_lib) should fix this issue
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's not really needed. It just looked more consistent with the rest of this patch.
fa0fce1 to
7232ef5
Compare
This allows CMAKE to wrap the executable with -DCMAKE_CROSSCOMPILING_EMULATOR=... Signed-off-by: Zoltán Böszörményi <zboszor@gmail.com>
5cfb59c to
6a6ab80
Compare
fa44cc1 to
7aef244
Compare
3c72253 to
a4888b3
Compare
7a09c51 to
93e941f
Compare