-
Notifications
You must be signed in to change notification settings - Fork 24.9k
[OpenReg] Add OSX/Windows Support for OpenReg #159441
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
Open
FFFrog
wants to merge
9
commits into
pytorch:main
Choose a base branch
from
FFFrog:platform
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from all commits
Commits
Show all changes
9 commits
Select commit
Hold shift + click to select a range
1689f44
[OpenReg] Add OSX/Windows Support for OpenReg
FFFrog a9194c6
Update Windows compilation for OpenReg
can-gaa-hou 5ee0ad5
Fix some issues
can-gaa-hou 4c41abc
Add torch_python_library
can-gaa-hou ff890b3
Update openreg testcase for Windows
can-gaa-hou 71dc569
Removing libprotobuf from cmake
can-gaa-hou 95279ed
Fix some issues
can-gaa-hou 7381aa4
Fix lint error
can-gaa-hou 65f2cc0
Fix some issues in comments
can-gaa-hou File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
20 changes: 20 additions & 0 deletions
20
test/cpp_extensions/open_registration_extension/torch_openreg/cmake/TorchPythonTargets.cmake
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,20 @@ | ||
if(WIN32) | ||
set(TORCH_PYTHON_IMPORTED_LOCATION "${PYTORCH_INSTALL_DIR}/lib/torch_python.lib") | ||
else() | ||
set(TORCH_PYTHON_IMPORTED_LOCATION "${PYTORCH_INSTALL_DIR}/lib/libtorch_python.so") | ||
endif() | ||
|
||
add_library(torch_python SHARED IMPORTED) | ||
|
||
set_target_properties(torch_python PROPERTIES | ||
INTERFACE_INCLUDE_DIRECTORIES "${PYTORCH_INSTALL_DIR}/include" | ||
INTERFACE_LINK_LIBRARIES "c10;torch_cpu" | ||
IMPORTED_LOCATION "${TORCH_PYTHON_IMPORTED_LOCATION}" | ||
) | ||
|
||
add_library(torch_python_library INTERFACE IMPORTED) | ||
|
||
set_target_properties(torch_python_library PROPERTIES | ||
INTERFACE_INCLUDE_DIRECTORIES "\$<TARGET_PROPERTY:torch_python,INTERFACE_INCLUDE_DIRECTORIES>" | ||
INTERFACE_LINK_LIBRARIES "\$<TARGET_FILE:torch_python>;\$<TARGET_PROPERTY:torch_python,INTERFACE_LINK_LIBRARIES>" | ||
) |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
14 changes: 10 additions & 4 deletions
14
.../cpp_extensions/open_registration_extension/torch_openreg/csrc/runtime/OpenRegFunctions.h
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,16 +1,22 @@ | ||
#pragma once | ||
|
||
#ifdef _WIN32 | ||
#define OPENREG_EXPORT __declspec(dllexport) | ||
#else | ||
#define OPENREG_EXPORT __attribute__((visibility("default"))) | ||
#endif | ||
|
||
#include <c10/core/Device.h> | ||
#include <c10/macros/Macros.h> | ||
|
||
#include <limits> | ||
|
||
namespace c10::openreg { | ||
|
||
c10::DeviceIndex device_count() noexcept; | ||
DeviceIndex current_device(); | ||
void set_device(c10::DeviceIndex device); | ||
OPENREG_EXPORT c10::DeviceIndex device_count() noexcept; | ||
OPENREG_EXPORT c10::DeviceIndex current_device(); | ||
OPENREG_EXPORT void set_device(c10::DeviceIndex device); | ||
|
||
DeviceIndex ExchangeDevice(DeviceIndex device); | ||
OPENREG_EXPORT DeviceIndex ExchangeDevice(DeviceIndex device); | ||
|
||
} // namespace c10::openreg |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
Hey @albanD, sorry to bother you.
PyTorch provides some CMake targets, such as
torch_cpu_library
andtorch_cuda_library
, but it doesn't provide a CMake target fortorch_python
, which is very useful for out-of-tree backends, as they require some Python bindings. We currently wrap a CMake target fortorch_python
in the openreg project, but it would be even better if Torch itself could provide 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.
Ho that is interesting.
I would have expected that our torch.utils.cpp_extension uses a cmake target to build these cpp extensions. But it doesn't?
Uh oh!
There was an error while loading. Please reload this page.
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.
As far as I know, basic functions in
torch.utils.cpp_extension
such asCPPExtension
use-ltorch_python
and specifylibrary_dir
directly to build the extension instead of using cmake targets.pytorch/torch/utils/cpp_extension.py
Lines 1204 to 1210 in 1ca8388
In addition, requirements of
torch.utils.cpp_extension
and integration of the new accelerator are slightly different; the core functions of the former are basically compiled into the extension library itself (such as _C.so), while the latter extension library itself is just an intermediate layer from Python to C++, and the core functions are all in the accelerator's own torch_binding.so.torch.uitls.cpp_extension
is more suitable for PyTorch users to extend some functions of torch (such as Pluggable Allocator), but it is not suitable for the latter. A highly integrated CMake object liketorch_python_library
is what the latter needs.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.
Ok!
Adding a new target sounds good then.
cc @swolchok can you give a quick look at the new cmake target? Is that the right style that we want going forward?