8000 Remove the SYCL hacks by rafbiels · Pull Request #1160 · g-truc/glm · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

Remove the SYCL hacks #1160

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

Merged
merged 1 commit into from
Nov 9, 2023
Merged

Conversation

rafbiels
Copy link

This reverts PR #914 which introduced a hacky way to replace all std namespace maths function calls with sycl namespace ones.

Presumably the original intention was to use GLM functions in SYCL device code (e.g. on GPUs) and force it to use the maths implementations optimised for the target device. However, this has been very limited in scope since the start because GLM relies heavily on function pointers which are illegal to use inside SYCL device code. So a lot of GLM cannot be used this way in any case.

The hacky solution shadowing std namespace with glm::std is problematic in many ways. One was that it required re-introducing all std symbols used across GLM codebase back to glm::std. The list of these symbols is difficult to maintain over time without extensive CI testing and unsurprisingly it got broken. Any code just including (some of) GLM headers now no longer compiles with SYCL compilers even if GLM is only used on the host side (CPU code).

Remove this hack to allow SYCL programs using GLM on the host side to compile.

The original hack was tested against the ComputeCpp compiler which is now phased out in favour of Intel's DPC++. Remove also the mention of ComputeCpp from README. The statement about "any C++11 compiler" still covers the host code compilation with DPC++.

This PR fixes the compilation and execution of the GLM test suite with the DPC++ SYCL compiler (icpx):

$ CXX=icpx cmake -DCMAKE_CXX_FLAGS="-fsycl -fno-fast-math" ../glm
$ cmake --build .
$ ctest

as well as example programs like:

$ cat test.cpp 
#include <glm/glm.hpp>
#include <glm/ext.hpp>
int main(){} 
$ icpx -fsycl -I../glm test.cpp

This reverts PR g-truc#914 which introduced a hacky way to replace
all std namespace maths function calls with sycl namespace ones.

Presumably the original intention was to use GLM functions in SYCL
device code (e.g. on GPUs) and force it to use the maths implementations
optimised for the target device. However, this has been very limited
in scope since the start because GLM relies heavily on function pointers
which are illegal to use inside SYCL device code.

The hacky solution shadowing std namespace with glm::std is problematic
in many ways. One was that it required re-introducing all std symbols used
across GLM codebase back to glm::std. The list of these symbols is difficult
to maintain over time without extensive CI testing and unsurprisingly it got
broken. Any code just including (some of) GLM headers now no longer compiles
with SYCL compilers even if GLM is only used on the host side (CPU code).

Remove this hack to allow SYCL programs using GLM on the host side to compile.

The original hack was tested against the ComputeCpp compiler which is now
phased out in favour of Intel's DPC++. Remove also the mention of ComputeCpp
from README. The statement about "any C++11 compiler" still covers the host
code compilation with DPC++.
@christophe-lunarg christophe-lunarg merged commit 586a402 into g-truc:master Nov 9, 2023
@christophe-lunarg
Copy link

Thanks for contributing

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants
0