make codebase portable #167
Conversation
specifically for MSVC
There was a problem hiding this comment.
mostly fine now, but some stuff is weird.
I need another opinion on this
example_saxpy segfaults directly when touching main() which doesn't make much sense for me and since there is no call stack idk how to even begin debugging that
example_program has the same experimental/iterator problem
| if (CMAKE_CXX_COMPILER_ID STREQUAL "MSVC") | ||
| add_library(yacx STATIC ${SOURCES}) | ||
| else() | ||
| add_library(yacx SHARED ${SOURCES}) | ||
| endif() |
There was a problem hiding this comment.
not sure if we should just stick to STATIC for all compilers?
|
|
||
| include_directories(include) | ||
| include_directories(include/yacx) | ||
| include_directories(${CMAKE_CUDA_TOOLKIT_INCLUDE_DIRECTORIES}) | ||
|
|
||
| link_directories("${CMAKE_CUDA_TOOLKIT_INCLUDE_DIRECTORIES}/../lib") | ||
| if (CMAKE_CXX_COMPILER_ID STREQUAL "MSVC") | ||
| link_directories("${CMAKE_CUDA_TOOLKIT_INCLUDE_DIRECTORIES}/../lib/x64") |
There was a problem hiding this comment.
looks weird, but works? not sure why this is necessary since link_libraries should be enough and is enough for nvrtc
| { | ||
| // See https://go.microsoft.com//fwlink//?linkid=834763 for more information about this file. | ||
| "configurations": [ | ||
| { | ||
| "name": "x64-Debug", | ||
| "generator": "Ninja", | ||
| "configurationType": "Debug", | ||
| "inheritEnvironments": [ "msvc_x64_x64" ], | ||
| "buildRoot": "${projectDir}\\out\\build\\${name}", | ||
| "installRoot": "${projectDir}\\out\\install\\${name}", | ||
| "cmakeCommandArgs": "", | ||
| "buildCommandArgs": "-v", | ||
| "ctestCommandArgs": "" | ||
| } | ||
| ] | ||
| } |
There was a problem hiding this comment.
maybe remove this file not sure
| @@ -5,7 +5,7 @@ using yacx::loglevel; | |||
| int main(int argc, char const *const *const argv) { | |||
| yacx::handle_logging_args(argc, argv); | |||
|
|
|||
| Logger(loglevel::ERROR) << "error message"; | |||
| Logger(loglevel::ERR) << "error message"; | |||
There was a problem hiding this comment.
this sucks so bad, but msvc somewhere does this:
#define ERROR 0There was a problem hiding this comment.
Guess I haven't yet changed it on the java side: https://github.com/ZerataX/yacx/pull/167/checks?check_run_id=659159835#step:3:218
There was a problem hiding this comment.
ich finde das ja sehr witzig, dass das schon vordefiniert ist
| @@ -137,7 +137,7 @@ int main() { | |||
|
|
|||
| // 2B. Set kernel string, header files and compile options | |||
| Headers headers; | |||
| headers.insert(Header{"cuda_runtime.h"}); | |||
| // headers.insert(Header{"cuda_runtime.h"}); | |||
There was a problem hiding this comment.
I'm actually really unsure why this was here to begin with?
But I also can't find this file automatically under windows like I can under linux, so I just removed it.
| @@ -109,7 +109,7 @@ class Logger { | |||
| current_loglevel = severity; | |||
| if (current_loglevel <= limit) { | |||
| std::stringstream prefix_ss; | |||
| prefix_ss << detail::get_color(severity) << get_datetime() << " " | |||
| prefix_ss /*<< get_color(severity)*/ << get_datetime() << " " | |||
There was a problem hiding this comment.
I do not understand the error here, please help @LukasSiefke
There was a problem hiding this comment.
Was ist denn die Fehlermeldung?
There was a problem hiding this comment.
eh so ein hardcore segfault, weil irg wo bei xstring irg eine funktion out of bounds geht oder so
| @@ -14,6 +17,7 @@ using yacx::loglevel, yacx::detail::dynop, yacx::detail::opfn; | |||
|
|
|||
| void yacx::detail::load_op(struct dynop *dest, std::string filename, std::string opSymbolName) { | |||
| //open libary | |||
| #ifndef _MSC_VER | |||
There was a problem hiding this comment.
not the most elegant solution, but linking libraries is vastly different under windows. we could use something like this: https://github.com/alainfrisch/flexdll
not sure how to properly say "hey these functions don't work under windows" so for now they just do nothing
There was a problem hiding this comment.
@LukasSiefke this is so sad
There was a problem hiding this comment.
sollte wahrscheinlich halt ne not implemented exception werfen oder so
| @@ -64,7 +65,7 @@ TEST_CASE("sumArray with implicit size", "[example_program]") { | |||
| double epsilon = 1.0E-8; | |||
| for (int i = 0; i < nElem; ++i) { | |||
| if (abs(hostRef[i] - gpuRef[i]) > epsilon) { | |||
| char *error_string; | |||
| char *error_string; | |||
There was a problem hiding this comment.
I guess I should run clang format over this once.
|
https://github.com/ZerataX/yacx/pull/167/checks?check_run_id=659121385#step:3:91 Edit: yeah it did and I fixed it |
|
some of the tests also fail, mostly due to: where again I wonder if this is actually necessary at all. Also of course lspci for getting the device name won't work under windows Not really big stuff |
| #if defined(_MSC_VER) | ||
| std::string name = exec("powershell -command \"wmic path win32_VideoController get name | " | ||
| "Select-String -Pattern 'NVIDIA ([a-zA-Z0-9\-]+)' " | ||
| "-AllMatches | % { $_.matches.groups[1].value }\""); | ||
| #else |
There was a problem hiding this comment.
not exactly the same regex pattern, but I really don't remember why I used a lookahead stuff or w/e there. idk seems to work like this.
There was a problem hiding this comment.
don't think powershell regex supports lookahead/behind
| @@ -33,15 +33,13 @@ TEST_CASE( | |||
| args.emplace_back(KernelArg(&datasize)); | |||
|
|
|||
| Headers headers; | |||
There was a problem hiding this comment.
I guess I could have just removed the Headers headers as well, but seems fine to me like this.
specifically for MSVC and GCC