-
Notifications
You must be signed in to change notification settings - Fork 787
[SYCL][libdevice] Refactor cmake for imf SYCL device library #6312
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
[SYCL][libdevice] Refactor cmake for imf SYCL device library #6312
Conversation
Signed-off-by: jinge90 <[email protected]>
Signed-off-by: jinge90 <[email protected]>
Signed-off-by: jinge90 <[email protected]>
Signed-off-by: jinge90 <[email protected]>
Signed-off-by: jinge90 <[email protected]>
Signed-off-by: jinge90 <[email protected]>
Signed-off-by: jinge90 <[email protected]>
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.
LGTM
@intel/llvm-reviewers-runtime , who is fluent enough in this area to provide meaningful approval? |
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.
Looks appropriate
Hi, @pvchupin |
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.
Cmake part looks good.
@tcreech-intel, @zhaomaosu, could you please review? |
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.
I think this looks good.
Another approach might be to concatenate with the preprocessor, e.g., by introducing a top-level source which consists only of #include "imf_source.cpp"
lines. This could either be generated by CMake from imf_f32_fallback_src_list
and imf_fp64_fallback_src_list
or checked into sources. In the latter case the CMake is greatly simplified. I don't necessarily prefer this, but thought it might be worth mentioning.
Thanks Pavel |
@jinge90, please address post-commit issues: https://github.com/intel/llvm/actions/runs/2518694462 |
…device (#6324) This patch aims to fix post-commit issue for #6312 Building libimf SYCL device library depends on sycl-compiler Signed-off-by: jinge90 <[email protected]>
Signed-off-by: jinge90 [email protected]
This PR aims to refactor cmake building system for imf SYCL device library. Previously, the source of SYCL fallback device library has pre-assumption that all functions in the same fallback .spv or .o file must reside in single .cpp file. The reason is we need to use sycl compiler to build .spv or .o file with following commands:
clang++ -fsycl -fsycl-device-only -fno-sycl-use-bitcode t.cpp -o t.spv
clang++ -fsycl -c -fsycl-targets="..." t.cpp -o t.o
However, imf fallback sources broke the pre-assumptions, all implementations reside in multiple .cpp files. Current building have to deal with multiple tools, for spv building, we have to do following:
For .o file, the steps are more complicated, we need to do following:
Current solution has following drawbacks:
So, we decide to use following solution:
For building imf fallback spv or object file, we first combine all required .cpp into a “full” .cpp and then use simple “clang++ -fsycl …” command to build it. We will create a “libdevice” folder in build/lib and save the full .cpp file there.
All imf fallback device library code will be organized to avoid code conflict, duplicate.