-
Notifications
You must be signed in to change notification settings - Fork 261
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat(bazel): support building C++ libs on windows platform #1873
Conversation
Signed-off-by: Junduo Dong <[email protected]>
Signed-off-by: Junduo Dong <[email protected]>
Signed-off-by: Junduo Dong <[email protected]>
Signed-off-by: Junduo Dong <[email protected]>
Signed-off-by: Junduo Dong <[email protected]>
Signed-off-by: Junduo Dong <[email protected]>
Have tried my best already : ( but there are still strange compiling errors using MSVC toolchain. It looks like some micro(s) cannot be compiled by MSVC correctly, right?
|
Another approach is replacing MSVC with Clang (related commit) but failed, either.
Maybe we need configure more details about the clang toolchain. : ( |
Based on the committed work, we could build the fury lib manually. (That is, commands in Build Fury C++ can be executed successfully) @chaokunyang Looking forward to your advice.. |
Looks strange. @PragmaTwice do you know where goes wrong? |
I don't have a windows machine for try this out, I will see whether I can reproduce this error by ssh into github windows CI machine |
You can use ERROR: D:/a/fury/fury/cpp/fury/thirdparty/BUILD:3:11: Compiling cpp/fury/thirdparty/MurmurHash3.cc failed: (Exit 2): cl.exe failed: error executing CppCompile command (from target //cpp/fury/thirdparty:libmmh3) C:\Program Files\Microsoft Visual Studio\2022\Enterprise\VC\Tools\MSVC\14.41.34120\bin\HostX64\x64\cl.exe @bazel-out/x64_windows-opt/bin/cpp/fury/thirdparty/_objs/libmmh3/MurmurHash3.obj.params
cpp/fury/thirdparty/MurmurHash3.cc(129): error C3861: '_rotl': identifier not found
cpp/fury/thirdparty/MurmurHash3.cc(133): error C3861: '_rotl': identifier not found
cpp/fury/thirdparty/MurmurHash3.cc(152): error C3861: '_rotl': identifier not found
cpp/fury/thirdparty/MurmurHash3.cc(196): error C3861: '_rotl': identifier not found
cpp/fury/thirdparty/MurmurHash3.cc(200): error C3861: '_rotl': identifier not found
cpp/fury/thirdparty/MurmurHash3.cc(205): error C3861: '_rotl': identifier not found
cpp/fury/thirdparty/MurmurHash3.cc(209): error C3861: '_rotl': identifier not found
cpp/fury/thirdparty/MurmurHash3.cc(214): error C3861: '_rotl': identifier not found
cpp/fury/thirdparty/MurmurHash3.cc(218): error C3861: '_rotl': identifier not found
cpp/fury/thirdparty/MurmurHash3.cc(223): error C3861: '_rotl': identifier not found
cpp/fury/thirdparty/MurmurHash3.cc(227): error C3861: '_rotl': identifier not found
cpp/fury/thirdparty/MurmurHash3.cc(250): error C3861: '_rotl': identifier not found
cpp/fury/thirdparty/MurmurHash3.cc(263): error C3861: '_rotl': identifier not found
cpp/fury/thirdparty/MurmurHash3.cc(276): error C3861: '_rotl': identifier not found
cpp/fury/thirdparty/MurmurHash3.cc(289): error C3861: '_rotl': identifier not found
cpp/fury/thirdparty/MurmurHash3.cc(350): error C3861: '_rotl64': identifier not found
cpp/fury/thirdparty/MurmurHash3.cc(354): error C3861: '_rotl64': identifier not found
cpp/fury/thirdparty/MurmurHash3.cc(359): error C3861: '_rotl64': identifier not found
cpp/fury/thirdparty/MurmurHash3.cc(363): error C3861: '_rotl64': identifier not found
cpp/fury/thirdparty/MurmurHash3.cc(392): error C3861: '_rotl64': identifier not found
cpp/fury/thirdparty/MurmurHash3.cc(413): error C3861: '_rotl64': identifier not found I added
|
I don't think our current macro definition works for windows: #define FURY_PP_NARG_IMPL(...) FURY_PP_NARG_CALC(__VA_ARGS__)
#define FURY_PP_NARG_CALC( \
_1, _2, _3, _4, _5, _6, _7, _8, _9, _10, _11, _12, _13, _14, _15, _16, \
_17, _18, _19, _20, _21, _22, _23, _24, _25, _26, _27, _28, _29, _30, _31, \
_32, _33, _34, _35, _36, _37, _38, _39, _40, _41, _42, _43, _44, _45, _46, \
_47, _48, _49, _50, _51, _52, _53, _54, _55, _56, _57, _58, _59, _60, _61, \
_62, _63, N, ...) \
N may need to change to(MSVC needs additional expansion pass) : #define FURY_PP_EXPAND(x) x
#define FURY_PP_NARG_IMPL(...) \
FURY_PP_EXPAND(FURY_PP_NARG_CALC(__VA_ARGS__, \
63, 62, 61, 60, 59, 58, 57, 56, 55, 54, 53, 52, 51, 50, \
49, 48, 47, 46, 45, 44, 43, 42, 41, 40, 39, 38, 37, 36, \
35, 34, 33, 32, 31, 30, 29, 28, 27, 26, 25, 24, 23, 22, \
21, 20, 19, 18, 17, 16, 15, 14, 13, 12, 11, 10, 9, 8, \
7, 6, 5, 4, 3, 2, 1, 0))
#define FURY_PP_NARG_CALC( \
_1, _2, _3, _4, _5, _6, _7, _8, _9, _10, \
_11, _12, _13, _14, _15, _16, _17, _18, _19, _20, \
_21, _22, _23, _24, _25, _26, _27, _28, _29, _30, \
_31, _32, _33, _34, _35, _36, _37, _38, _39, _40, \
_41, _42, _43, _44, _45, _46, _47, _48, _49, _50, \
_51, _52, _53, _54, _55, _56, _57, _58, _59, _60, \
_61, _62, _63, N, ...) N Could you try to make cpp/fury/meta/preprocessor.h support windows? @An-DJ |
Signed-off-by: Junduo Dong <[email protected]>
Signed-off-by: Junduo Dong <[email protected]>
Signed-off-by: Junduo Dong <[email protected]>
@chaokunyang According to related documents, we can add Thanks to Microsoft : ) |
Signed-off-by: Junduo Dong <[email protected]>
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 awesome, seems we can support pyfury for Windows now. Thank you @An-DJ
What does this PR do?
Make bazel happy on
Windows OS
.row.cc
, then it could be compiled by MSVC (VLA is not supported)/Zc:preprocessor
to enable C99/C11/utf-8
to set source and execution character sets to UTF-8.*.lib
to the linkerwindows-2022
to build fury cpp librariesRelated issues
#798
Does this PR introduce any user-facing change?
Benchmark