-
Notifications
You must be signed in to change notification settings - Fork 801
[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 <ge.jin@intel.com>
Signed-off-by: jinge90 <ge.jin@intel.com>
Signed-off-by: jinge90 <ge.jin@intel.com>
Signed-off-by: jinge90 <ge.jin@intel.com>
Signed-off-by: jinge90 <ge.jin@intel.com>
Signed-off-by: jinge90 <ge.jin@intel.com>
Signed-off-by: jinge90 <ge.jin@intel.com>
xtian-github
left a comment
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? |
akolesov-nv
left a comment
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 |
zettai-reido
left a comment
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? |
tcreech-intel
left a comment
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 |
Signed-off-by: jinge90 ge.jin@intel.com
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.