Repository navigation
[SYCL] Support non-x86 platforms - #2333
Conversation
|
@jeffhammond is this enough to build the runtime library on |
There was a problem hiding this comment.
This is going to produce warning any time when code is compiled for non-x86 platform, right? But patch enables the support of non-x64 platform, so this is a bit confusing.
There was a problem hiding this comment.
The warning is meant to indicate that the specific function is not supported. Should I change the warning? It is very hard to determine the CPU frequency on an ARM platform and the only thing I found requires a Linux 5.x kernel and read access to /sys...
There was a problem hiding this comment.
I see, I am ok to not support this function for ARM.
If someone compiles this for ARM and calls this function 0 is going to be returned, should we also throw runtime error or compile-time warning is enough?
There was a problem hiding this comment.
Currently, this programs throws the following error unconditionally, but I figure that will be removed at some point so I decided to add a placeholder for a non-x86 implementation, for whenever somebody figures out how to get the frequency of an ARM CPU.
throw runtime_error(
"max_clock_frequency parameter is not supported for host device",
PI_INVALID_DEVICE);
Great. This should help closing the oldest open issue #4. Although I'm not sure if @kpet implied adding support for these functions on ARM or just building the library is enough. |
tested on AAarch64: - Cavium ThunderX2 with Linux 4.15 - Raspberry Pi 4 with Linux 5.5 Signed-off-by: Jeff R. Hammond <[email protected]>
Signed-off-by: Hammond, Jeff R <[email protected]>
4971ef0 to
e44b026
Compare
Signed-off-by: Hammond, Jeff R <[email protected]>
MSVC does not support #warning compile-time warnings about non-support are not that useful. the code path that generated the warning should not break anything, it just might not be optimal and accurately reflect the underlying platform. Signed-off-by: Hammond, Jeff R <[email protected]>
|
I resolved the remaining outstanding feedback. Is this ready for merging? |
bader
left a comment
There was a problem hiding this comment.
@jeffhammond, please, fix the build failure on Windows.
D:\buildbot\Product_worker_intel\sycl-win-x64-pr\llvm.src\sycl\source\detail\platform_util.cpp(109): error C2220: warning treated as error - no 'object' file generated
D:\buildbot\Product_worker_intel\sycl-win-x64-pr\llvm.src\sycl\source\detail\platform_util.cpp(109): warning C4189: 'Index': local variable is initialized but not referenced
D:\buildbot\Product_worker_intel\sycl-win-x64-pr\llvm.src\sycl\source\detail\platform_util.cpp(177): error C2065: '_MM_HINT_T0': undeclared identifier
D:\buildbot\Product_worker_intel\sycl-win-x64-pr\llvm.src\sycl\source\detail\platform_util.cpp(177): error C3861: '_mm_prefetch': identifier not found
Signed-off-by: Hammond, Jeff R <[email protected]>
|
@bader I think I fixed it, but since I do not use Windows, I cannot verify. |
|
Let's wait for BuildBot results. |
Tidy a string in an options test
tested on AAarch64:
Signed-off-by: Jeff R. Hammond [email protected]