-
Notifications
You must be signed in to change notification settings - Fork 83
CMake generates MSVC 2015 solutions for ParallelSTL #41
base: main
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you update the main Readme file to reflect the support of Visual Studio builds and with instructions on how use it?
@@ -36,7 +36,10 @@ include_directories("include") | |||
|
|||
add_subdirectory (src) | |||
add_subdirectory (examples) | |||
add_subdirectory (tests) | |||
|
|||
if (NOT WIN32) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Add a comment explaining why tests are disabled on windows
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Add a comment explaining why tests are disabled
on windows
I'm partial to changing this from a Windows thing to all systems. If you don't have the tests pre-installed on any system, then you can't actually run them.
Comment still valid, just needs to be broadened.
|
||
endfunction(add_sycl_to_target) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this different from the module in the computecpp-sdk? If so we can possible update the sdk one?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is actually copied from the SDK to here!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
good then :-)
elseif (targetCxxStandard MATCHES 11) | ||
set(device_compiler_cxx_standard "-std=c++11") | ||
elseif (targetCxxStandard MATCHES 98) | ||
message(FATAL_ERROR "SYCL implementations cannot be compiled using C++98") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
SYCL implementations -> SYCL applications
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good, general comments
README.md
Outdated
|
||
```bash | ||
sudo apt update && sudo apt upgrade && sudo apt update | ||
sudo apt install build-essentials binutils gdb git flex bison texlive-full |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why do you need flex and bison?
cd CMake | ||
./bootstrap && make -j 4 && sudo make install | ||
cd .. | ||
sudo apt install ocl-icd-libopencl1 ocl-icd-dev opencl-headers clinfo lsb-core |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
clinfo is not required as the user can run computecpp_info to get the OpenCL devices. this is also more relevant as the user can check which devices work with computeCpp
else() | ||
message(FATAL_ERROR "libComputeCpp.so - Not found!") | ||
message(FATAL_ERROR "ComputeCpp Runtime Library - Not found!") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would prefer to know what is the file name as well
If any comments come up about FindComputeCpp.cmake, could you make a PR to the SDK so we can change it there? Keeps the flow unidirectional which I think will make things a lot cleaner. Thanks! |
MSVC++ doesn't support C++11. Pending Clang 6 for C++14, as Microsoft don't support Clang 3.9. |
Christopher Di Bella seems not to be a GitHub user. You need a GitHub account to be able to sign the CLA. If you have already a GitHub account, please add the email address used for this commit to your account. You have signed the CLA already but the status is still pending? Let us recheck it. |
CMake generates an MSVC 2015 x64 ParallelSTL solution that successfully compiles using the configuration in this branch.