Skip to content

Fix missing unistd.h on Windows #152

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.

Sign up for GitHub

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

Merged
merged 6 commits into from
Jul 29, 2020
Merged

Fix missing unistd.h on Windows #152

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
53c3430
Update main.cpp
xsacha Jul 20, 2020
1a00287
Include missing export on ExtraInfo::DataType
srefshauge Jul 20, 2020
c98832d
Add Windows support to Bazel scripts.
srefshauge Jul 22, 2020
6150abe
Changes as requested: #if/#else
srefshauge Jul 23, 2020
4ade7eb
Missing commas in tensorrt bazel
srefshauge Jul 23, 2020
6346b1c
Fix logging on MSVC
Jul 29, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 14 additions & 0 deletions core/util/macros.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,13 +25,27 @@
#define LOG_ERROR_OWN(l,s) TRTORCH_LOG(l, core::util::logging::LogLevel::kERROR, s)
#define LOG_INTERNAL_ERROR_OWN(l,s) TRTORCH_LOG(l, core::util::logging::LogLevel::kINTERNAL_ERROR, s)

#ifdef _MSC_VER

#define EXPAND( x ) x

#define LOG_GRAPH(...) EXPAND(GET_MACRO(__VA_ARGS__, LOG_GRAPH_OWN, LOG_GRAPH_GLOBAL)(__VA_ARGS__))
#define LOG_DEBUG(...) EXPAND(GET_MACRO(__VA_ARGS__, LOG_DEBUG_OWN, LOG_DEBUG_GLOBAL)(__VA_ARGS__))
#define LOG_INFO(...) EXPAND(GET_MACRO(__VA_ARGS__, LOG_INFO_OWN, LOG_INFO_GLOBAL)(__VA_ARGS__))
#define LOG_WARNING(...) EXPAND(GET_MACRO(__VA_ARGS__, LOG_WARNING_OWN, LOG_WARNING_GLOBAL)(__VA_ARGS__))
#define LOG_ERROR(...) EXPAND(GET_MACRO(__VA_ARGS__, LOG_ERROR_OWN, LOG_ERROR_GLOBAL)(__VA_ARGS__))
#define LOG_INTERNAL_ERROR(...) EXPAND(GET_MACRO(__VA_ARGS__, LOG_INTERNAL_ERROR_OWN, LOG_INTERNAL_ERROR_GLOBAL)(__VA_ARGS__))

#else

#define LOG_GRAPH(...) GET_MACRO(__VA_ARGS__, LOG_GRAPH_OWN, LOG_GRAPH_GLOBAL)(__VA_ARGS__)
#define LOG_DEBUG(...) GET_MACRO(__VA_ARGS__, LOG_DEBUG_OWN, LOG_DEBUG_GLOBAL)(__VA_ARGS__)
#define LOG_INFO(...) GET_MACRO(__VA_ARGS__, LOG_INFO_OWN, LOG_INFO_GLOBAL)(__VA_ARGS__)
#define LOG_WARNING(...) GET_MACRO(__VA_ARGS__, LOG_WARNING_OWN, LOG_WARNING_GLOBAL)(__VA_ARGS__)
#define LOG_ERROR(...) GET_MACRO(__VA_ARGS__, LOG_ERROR_OWN, LOG_ERROR_GLOBAL)(__VA_ARGS__)
#define LOG_INTERNAL_ERROR(...) GET_MACRO(__VA_ARGS__, LOG_INTERNAL_ERROR_OWN, LOG_INTERNAL_ERROR_GLOBAL)(__VA_ARGS__)

#endif
// ----------------------------------------------------------------------------
// Error reporting macros
// ----------------------------------------------------------------------------
Expand Down
2 changes: 1 addition & 1 deletion cpp/api/include/trtorch/trtorch.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -90,7 +90,7 @@ struct TRTORCH_API ExtraInfo {
* This class is compatable with c10::DataTypes (but will check for TRT support)
* so there should not be a reason that you need to use this type explictly.
*/
class DataType {
class TRTORCH_API DataType {
public:
/**
* Underlying enum class to support the DataType Class
Expand Down
11 changes: 9 additions & 2 deletions cpp/trtorchc/main.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,14 +1,21 @@
#include <iostream>
#include <sstream>
#include <stdlib.h>
#include <unistd.h>

#ifdef linux
#include <linux/limits.h>
#else
#define PATH_MAX 260
#endif

#if defined(_WIN32)
#include <direct.h>
#define getcwd _getcwd
#define realpath(N,R) _fullpath((R),(N),PATH_MAX)
#else
#include <unistd.h>
#endif

#include "NvInfer.h"
#include "third_party/args/args.hpp"
#include "torch/torch.h"
Expand Down Expand Up @@ -366,4 +373,4 @@ int main(int argc, char** argv) {
}

return 0;
}
}
30 changes: 27 additions & 3 deletions third_party/cuda/BUILD
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,12 +8,22 @@ config_setting(
],
)

config_setting(
name = "windows",
constraint_values = [
"@platforms//os:windows",
],
)

cc_library(
name = "cudart",
srcs = select({
":aarch64_linux": [
"targets/aarch64-linux/lib/libcudart.so",
],
":windows": [
"lib/x64/cudart.lib",
],
"//conditions:default": [
"targets/x86_64-linux/lib/libcudart.so",
],
Expand All @@ -32,6 +42,9 @@ cc_library(
":aarch64_linux": [
"targets/aarch64-linux/lib/libnvToolsExt.so.1",
],
":windows": [
"bin/nvToolsExt64_1.dll",
],
"//conditions:default": [
"targets/x86_64-linux/lib/libnvToolsExt.so.1",
],
Expand All @@ -44,6 +57,9 @@ cc_library(
":aarch64_linux": glob([
"targets/aarch64-linux/lib/**/lib*.so",
]),
":windows": [
"bin/*.dll",
],
"//conditions:default": glob([
"targets/x86_64-linux/lib/**/lib*.so",
]),
Expand All @@ -59,9 +75,17 @@ cc_library(

cc_library(
name = "cublas",
srcs = glob([
"lib/**/*libcublas.so",
]),
srcs = select({
":aarch64_linux": glob([
"lib/**/*libcublas.so",
]),
":windows": glob([
"lib/x64/cublas.lib",
]),
"//conditions:default": glob([
"lib/**/*libcublas.so",
]),
}),
hdrs = glob([
"include/**/*cublas*.h",
"include/**/*.hpp",
Expand Down
8 changes: 8 additions & 0 deletions third_party/cudnn/local/BUILD
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,13 @@ config_setting(
],
)

config_setting(
name = "windows",
constraint_values = [
"@platforms//os:windows",
],
)

cc_library(
name = "cudnn_headers",
hdrs = ["include/cudnn.h"] + glob(["include/cudnn+.h"]),
Expand All @@ -19,6 +26,7 @@ cc_import(
name = "cudnn_lib",
shared_library = select({
":aarch64_linux": "lib/aarch64-linux-gnu/libcudnn.so",
":windows": glob(["bin/cudnn64_*.dll"])[0],
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is kind of odd, is there a reason you chose glob here?

Copy link
Collaborator

@narendasan narendasan Jul 23, 2020 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just to note, the glob failed, I ended up just putting the 7 in the path and removing the glob

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How do we support cudnn8?
I was trying to make sure it supports v8 and the glob worked for me to pick 7 or 8.

I'm not good with bazel and couldn't figure out how to do a wildcard without a glob. Then a glob returns a list so I needed to get the first value.

Copy link
Contributor Author

@xsacha xsacha Jul 23, 2020 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also just curious when you do development on Windows do you typically install libraries like cudnn or tensorrt to the system somewhere and reuse them or do you keep them in a workspace that is project specific. Seems like we could extend these changes to the archive based dependencies too pretty easily. Doesn't have to be in this PR but just wondering if it would be useful.

I installed cudnn and tensorrt to a folder and specified it in WORKSPACE. I'm more of a Linux developer but need this working on Windows if we are to use it at all.

Not sure what's going on with the stream operator there. From the error, it looks like it doesn't like:

#define TRTORCH_LOG(l, sev, msg)               \
    do {                                       \
        std::stringstream ss{};                \
        ss << msg;                             \
        l.log(sev, ss.str());                  \
    } while (0)

Maybe a MSVC issue (if that's the compiler you are using).
If you remove the line:
ss << msg;
does it compile?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can look into if there is another way to get the wildcard working. Hmm, I can look into the logging issue more, you didn't hit it right? What version of MSVC did you use? Did you add any additional flags or anything (like I noticed MSVC doesnt handle default flags we have like --std=c++14)?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@narendasan Any update with this?
It seems to work fine here. Is it the version of Bazel I am using?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems like something about the particular use of a specialized logger in conversion.cpp that is causing the issue, If I switch to the generic logger I can compile the library and successfully start programs using the core library such as trtorchc (I don't notice the issue mentioned in #153 if i understood the issue correctly either, the compiler starts up correctly but I think we don't handle windows paths well so i haven't yet tested further). I dont think it is due to the bazel version as it just calls the underlying C++ compiler similar to how make or CMake does. I think its a compiler issue, I am not sure which compiler version you used, I used the latest MSVC 2019 build tools. If there is anymore information you could give me about your build environment and build process that would be helpful. I think other than that issue this PR is good to merge though.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I mean for the globbing issue. Is this related to the bazel version?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No I think it may have been a path error on my part i think. It works now.

"//conditions:default": "lib/x86_64-linux-gnu/libcudnn.so",
}),
visibility = ["//visibility:private"],
Expand Down
56 changes: 42 additions & 14 deletions third_party/libtorch/BUILD
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,12 @@
package(default_visibility = ["//visibility:public"])

config_setting(
name = "windows",
constraint_values = [
"@platforms//os:windows",
],
)

cc_library(
name = "libtorch",
deps = [
Expand All @@ -18,12 +25,20 @@ cc_library(
) + glob([
'include/torch/csrc/api/include/**/*.h'
]),
srcs = [
'lib/libtorch.so',
'lib/libtorch_cuda.so',
'lib/libtorch_cpu.so',
'lib/libtorch_global_deps.so',
],
srcs = select({
":windows": [
'lib/torch.lib',
'lib/torch_cuda.lib',
'lib/torch_cpu.lib',
'lib/torch_global_deps.dll',
],
"//conditions:default": [
'lib/libtorch.so',
'lib/libtorch_cuda.so',
'lib/libtorch_cpu.so',
'lib/libtorch_global_deps.so',
],
}),
deps = [
":ATen",
":c10_cuda",
Expand All @@ -39,7 +54,10 @@ cc_library(
hdrs = glob([
'include/c10/**/*.h'
]),
srcs = ["lib/libc10_cuda.so"],
srcs = select({
":windows": ["lib/c10_cuda.lib"],
"//conditions:default": ["lib/libc10_cuda.so"],
}),
strip_include_prefix = "include",
deps = [
":c10"
Expand All @@ -51,7 +69,10 @@ cc_library(
hdrs = glob([
'include/c10/**/*.h'
]),
srcs = ["lib/libc10.so"],
srcs = select({
":windows": ["lib/c10.lib"],
"//conditions:default": ["lib/libc10.so"],
}),
strip_include_prefix = "include",
)

Expand All @@ -68,11 +89,18 @@ cc_library(
hdrs = glob([
'include/caffe2/**/*.h'
]),
srcs = [
'lib/libcaffe2_nvrtc.so',
'lib/libcaffe2_detectron_ops_gpu.so',
'lib/libcaffe2_observers.so',
'lib/libcaffe2_module_test_dynamic.so'
],
srcs = select({
":windows": [
'lib/caffe2_nvrtc.lib',
'lib/caffe2_detectron_ops_gpu.lib',
'lib/caffe2_module_test_dynamic.lib'
],
"//conditions:default": [
'lib/libcaffe2_nvrtc.so',
'lib/libcaffe2_detectron_ops_gpu.so',
'lib/libcaffe2_observers.so',
'lib/libcaffe2_module_test_dynamic.so'
],
}),
strip_include_prefix = "include",
)
Loading