celerity / celerity-runtime

High-level C++ for Accelerator Clusters
https://celerity.github.io
MIT License
139 stars 18 forks source link

Add support for SimSYCL (and GCC / C++20 / Sanitizers) #238

Closed fknorr closed 7 months ago

fknorr commented 7 months ago

This adds SimSYCL as a third supported SYCL implementation. SimSYCL executes kernels synchronously on the host and therefore has support for AddressSanitizer and LeakSanitizer, which we can now use to find UB in Celerity from within our CI.

SimSYCL requires C++20, so our code had to be adapted in a few places and 3rd-party dependencies had to be upgraded to be compatible with the new standard.

Until now all our CI builds have been made with Clang derivatives, so I opted for performing the SimSYCL build with GCC to broaden our compiler support at the same time. This required some further patches, either to work around GCC bugs or to be compliant where Clang was overly permissive.

Finally, SimSYCL + AddressSanitizer actually managed to find two use-after-scope errors in the Celerity tests, which were fixed.

Possible follow-up changes

github-actions[bot] commented 7 months ago

Check-perf-impact results: (4c65f1399a47e0eb1340f63004745b17)

:question: No new benchmark data submitted. :question:
Please re-run the microbenchmarks and include the results if your commit could potentially affect performance.

fknorr commented 7 months ago

Github Actions seems to mark this PR as failing :x: when the SimSYCL runs emit any warning, so I resolved all of them:

Curiously, the hipSYCL builds still emit -Wunknown-cuda-version but this does not seem to affect the build status.

fknorr commented 7 months ago

LGTM, I'd maybe squash some of the commits before merging (e.g. all the dependency version bumps).

I'd rather keep those separate, they are all relevant to the "internal changelog".

It's pretty neat to see the times for "Build and Install Celerity" in the CI run for SimSYCL.

They do look good, but that's mostly due to ccache. Build times for the other SYCL impls are not much longer. Except hipSYCL + Ubuntu 23.04? I wonder what's going on there.