CMake generates MSVC 2015 solutions for ParallelSTL #41
Conversation
Ruyk
left a comment
Ruyk
left a comment
There was a problem hiding this comment.
Can you update the main Readme file to reflect the support of Visual Studio builds and with instructions on how use it?
| add_subdirectory (examples) | ||
| add_subdirectory (tests) | ||
|
|
||
| if (NOT WIN32) |
There was a problem hiding this comment.
Add a comment explaining why tests are disabled on windows
There was a problem hiding this comment.
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.
| $<$<NOT:$<OR:$<CONFIG:Debug>,$<CONFIG:RelWithDebInfo>>>:${COMPUTECPP_RUNTIME_LIBRARY}> | ||
| ${OpenCL_LIBRARIES}) | ||
|
|
||
| endfunction(add_sycl_to_target) No newline at end of file |
There was a problem hiding this comment.
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.
This is actually copied from the SDK to here!
| 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.
SYCL implementations -> SYCL applications
marios-codeplay
left a comment
marios-codeplay
left a comment
There was a problem hiding this comment.
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.
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.
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
| message(STATUS "libComputeCpp.so - Found") | ||
| else() | ||
| message(FATAL_ERROR "libComputeCpp.so - Not found!") | ||
| message(FATAL_ERROR "ComputeCpp Runtime Library - Not found!") |
There was a problem hiding this comment.
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. |
5 participants
CMake generates an MSVC 2015 x64 ParallelSTL solution that successfully compiles using the configuration in this branch.