Rename hipSYCL/Open SYCL target to AdaptiveCpp #21
Conversation
|
The pipeline failed because the CI-GPU image needs to be rebuilt. It cannot be currently done because the LRZ compute cloud is down |
ravil-mobile
force-pushed
the
ravil/opensycl
branch
from
20c3308
to
37cfb0e
Compare
There was a problem hiding this comment.
Thanks alot! I left a bunch of smaller comments regarding the build system etc. ... The main thing is maybe the naming, i.e. it is "Open SYCL", not "openSYCL". Cf. here: AdaptiveCpp/AdaptiveCpp#942 (comment)
DeviceMacros.h
Outdated
| #elif defined(DEVICE_HIPSYCL_LANG) | ||
| #error do not launch kernel with macros using hipsycl | ||
| #elif defined(DEVICE_OPENSYCL_LANG) | ||
| #error do not launch kernel with macros using opensycl |
There was a problem hiding this comment.
The official name is written with a space in between, see AdaptiveCpp/AdaptiveCpp#942 (comment) .
There was a problem hiding this comment.
Hi, I renamed OpenSYCL to Open SYCL in READMEs.
#elif defined(DEVICE_OPENSYCL_LANG)
It is C-macro. Spaces are not going to work here.
|
|
||
|
|
||
| def get_full_target_name(arch): | ||
| data = get_config() |
There was a problem hiding this comment.
Right now, get_config is getting called twice—but the result is the same. While that shouldn't cause many problems (I hope)... Wouldn't it be better only load the config once and then re-use it?
There was a problem hiding this comment.
I am calling this python script 3 times to get different outputs to stdout
# 1. to get Open SYCL include dit
execute_process(COMMAND "${Python3_EXECUTABLE}" ${CMAKE_CURRENT_LIST_DIR}/find-opensycl.py -i
OUTPUT_VARIABLE _OPENSYCL_INLCUDE_DIR)
# 2. to get the vendor name for the default device
execute_process(COMMAND "${Python3_EXECUTABLE}" ${CMAKE_CURRENT_LIST_DIR}/find-opensycl.py --vendor
OUTPUT_VARIABLE _OPENSYCL_VENDOR)
# 3. to get correct default target name
execute_process(COMMAND "${Python3_EXECUTABLE}" ${CMAKE_CURRENT_LIST_DIR}/find-opensycl.py -t -a "${DEVICE_ARCH}"
OUTPUT_VARIABLE _OPENSYCL_FULL_TARGET_NAME)
I don't want to execute a python script once and get all these output in one go
because, in this case, I will have to parse the resultant string in CMake which is tedious and ugly.
|
|
||
|
|
||
| def get_package_dir(): | ||
| exe_name = which("syclcc") |
There was a problem hiding this comment.
Maybe make the package dir a CMake variable? Just in case the syclcc in PATH is not the one that is wanted (or some environment variable is supposed to supersede this name), or not given at all.
There was a problem hiding this comment.
Hmmm. I would argue that the user is responsible for providing a correct syclcc. The executable is supposed to visible in the environment. Otherwise, nothing is going to get compiled.
* renamed `OpenSycl-config` to `OpenSYCLSettings`
ravil-mobile
force-pushed
the
ravil/opensycl
branch
3 times, most recently
from
261385a
to
a6dab74
Compare
ravil-mobile
force-pushed
the
ravil/opensycl
branch
from
a6dab74
to
35df300
Compare
|
fyi, please see here: AdaptiveCpp/AdaptiveCpp#999 Hi @illuhad. Thank you and @davschneller for pointing it out. Yes, I paused this PR and I am waiting for your team to rename the project. I hope it will happen soon. As for me, both FreeSYCL and LibreSYCL sound good. |
davschneller
changed the title
No description provided.