Addition of SYCL implementation of SLAMBench #13
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…make within /usr/share/
…hen CXX is set to syclcc. The SYCL code is currently still exactly the same as the cpp version.
…em::get_global().
…the OpenCL version which may have a signed/unsigned bug.
…qual to cpp results.
…is currently a problem for intel's sycl. Turning CMAKE_CXX_EXTENSIONS off in CMakeLists.txt ensures we obtain -std=c++11.
…al artifacts on intel sycl.
Accidentally left in some <<<<< HEAD from conflicts. And I also did some accidental space addition / removal in places that aren't functionally required and just add noise to the pull request. Also some minor artifacts from the reapplication of the patches in an order they technically never were applied. Signed-off-by: Andrew Gozillon andrew.gozillon@yahoo.com
bbodin
requested changes
Dec 16, 2019
There was a problem hiding this comment.
Thanks for this contribution, this is just amazing !!
I apologize I did not review earlier.
I would have approved directly, but some files should not have been modified:
kfusion/include/default_parameters.h
kfusion/src/reader.cpp
kfusion/src/mainQt.cpp
kfusion/src/benchmark.cpp
kfusion/qt/SlamBenchQt.cpp
kfusion/include/interface.h
kfusion/include/draw.h
Do you think you can do something with it ?
Best wishes,
Thanks !
I've updated the documentation to be more up-to-date, especially after the recent script updates. I've made an attempt at fixing the build system when CXX is set to syclcc. You should now be able to compile all modules when it's set. The main change was modfiyng the -DSYCL compiler definition to only be speciifed for the SYCL module. There was another modification required that links to some other syclcc modifications that had to be done. When using syclcc (and by default ComputeCpp) as the main compiler, it unfortunately doesn't come packaged with OpenMP libraries which means the OpenMP module will fail unless a user has self installed the OpenMP libraries. The fix for this requried the addition of a new "driver" argument for syclcc and the application of this argument for the OpenMP module when syclcc is in use (-syclcc-use-host). Which essentially says use the host compiler, which by default is g++. This means it'll compile smoothly as g++ contains omp headers and libraries.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This adds a new SYCL module for SLAMBench.
It currently works using ComputeCpp for CPU and GPU and Intels open source SYCL implementation for Intel GPU.
The compilation of the SYCL modules have an external dependency on: https://github.com/agozillon/syclcc which are a set of driver scripts that allow CMake to treat the SYCL compiler as it would treat a normal C++ compiler without the needed addition of any new SYCL specific compile arguments.
Some steps covering this have been added to the README.md. On the note of the README.md I promise the changes aren't as significant as they look from the source <-> source difference! You should be able to just hit the "Display the rich diff" button and it'll show a more realistic view of the changes made.
@pkeir was the main contributor of this patch as you can probably tell by the prolific commit history! So I'm tagging him to help with the discussion of the pull request.