[ET-VK] Enable dynamic operator registration #2305

jorgep31415 · 2024-03-07T22:32:13Z

Stack from ghstack (oldest at bottom):

This change follows

in the footsteps of Implement global shader registry #2222 for static initialization and
the popular TorchLibraryImpl for wrapping with macros.

https://www.internalfb.com/code/fbsource/[b6860acf0fd7a95224f2ed3f6fe48f699a9a45c0]/fbcode/caffe2/torch/library.h?lines=1004%2C1012-1026

Contributors can now write their operator and register them within the same file using REGISTER_OPERATORS + VK_REGISTER_OP(), as shown in Arithmetic.h/cpp.

Typically in Linux/Android C++ environments, the symbols corresponding to OperatorRegisterInit static instances are discarded since they aren't used for anything other than static initialization. Hence, we need to link_whole = True for the vulkan_graph_runtime library.

We update our Compute API tests to verify we can go through OperatorRegistry with proper static initialization.

Differential Revision: D54641117

This change follows 1. in the footsteps of #2222 for static initialization and 2. the popular `TorchLibraryImpl` for wrapping with macros. https://www.internalfb.com/code/fbsource/[b6860acf0fd7a95224f2ed3f6fe48f699a9a45c0]/fbcode/caffe2/torch/library.h?lines=1004%2C1012-1026 Contributors can now write their operator and register them within the same file using `REGISTER_OPERATORS` + `VK_REGISTER_OP()`, as shown in `Arithmetic.h/cpp`. Typically in Linux/Android C++ environments, the symbols corresponding to `OperatorRegisterInit` static instances are discarded since they aren't used for anything other than static initialization. Hence, we need to `link_whole = True` for the `vulkan_graph_runtime` library. We update our Compute API tests to verify we can go through `OperatorRegistry` with proper static initialization. Differential Revision: [D54641117](https://our.internmc.facebook.com/intern/diff/D54641117/) [ghstack-poisoned]

pytorch-bot · 2024-03-07T22:32:17Z

🔗 Helpful Links

🧪 See artifacts and rendered test results at hud.pytorch.org/pr/pytorch/executorch/2305

📄 Preview Python docs built from this PR

Note: Links to docs will display an error until the docs builds have been completed.

❌ 1 New Failure, 1 Unrelated Failure

As of commit 5675406 with merge base 47b837b ():

NEW FAILURE - The following job has failed:

Build documentation / build (buck2) / Build doc (gh)
RuntimeError: Command docker exec -t b078287b65ca68525e5e6f4c2184df8574375cc987fd6729ca8431e0b3a96373 /exec failed with exit code 2

BROKEN TRUNK - The following job failed but were present on the merge base:

👉 Rebase onto the `viable/strict` branch to avoid these failures

Lint / lintrunner / linux-job (gh)
>>> Lint for examples/models/llama2/export_llama_lib.py:

This comment was automatically generated by Dr. CI and updates every 15 minutes.

facebook-github-bot · 2024-03-07T22:32:28Z

This pull request was exported from Phabricator. Differential Revision: D54641117

This change follows 1. in the footsteps of #2222 for static initialization and 2. the popular `TorchLibraryImpl` for wrapping with macros. https://www.internalfb.com/code/fbsource/[b6860acf0fd7a95224f2ed3f6fe48f699a9a45c0]/fbcode/caffe2/torch/library.h?lines=1004%2C1012-1026 Contributors can now write their operator and register them within the same file using `REGISTER_OPERATORS` + `VK_REGISTER_OP()`, as shown in `Arithmetic.h/cpp`. Typically in Linux/Android C++ environments, the symbols corresponding to `OperatorRegisterInit` static instances are discarded since they aren't used for anything other than static initialization. Hence, we need to `link_whole = True` for the `vulkan_graph_runtime` library. We update our Compute API tests to verify we can go through `OperatorRegistry` with proper static initialization. Differential Revision: [D54641117](https://our.internmc.facebook.com/intern/diff/D54641117/) ghstack-source-id: 217857140 Pull Request resolved: #2305

This change follows 1. in the footsteps of #2222 for static initialization and 2. the popular `TorchLibraryImpl` for wrapping with macros. https://www.internalfb.com/code/fbsource/[b6860acf0fd7a95224f2ed3f6fe48f699a9a45c0]/fbcode/caffe2/torch/library.h?lines=1004%2C1012-1026 Contributors can now write their operator and register them within the same file using `REGISTER_OPERATORS` + `VK_REGISTER_OP()`, as shown in `Arithmetic.h/cpp`. Typically in Linux/Android C++ environments, the symbols corresponding to `OperatorRegisterInit` static instances are discarded since they aren't used for anything other than static initialization. Hence, we need to `link_whole = True` for the `vulkan_graph_runtime` library. We update our Compute API tests to verify we can go through `OperatorRegistry` with proper static initialization. Differential Revision: [D54641117](https://our.internmc.facebook.com/intern/diff/D54641117/) [ghstack-poisoned]

facebook-github-bot · 2024-03-07T23:19:28Z

This pull request was exported from Phabricator. Differential Revision: D54641117

Pull Request resolved: #2305 This change follows 1. in the footsteps of #2222 for static initialization and 2. the popular `TorchLibraryImpl` for wrapping with macros. https://www.internalfb.com/code/fbsource/[b6860acf0fd7a95224f2ed3f6fe48f699a9a45c0]/fbcode/caffe2/torch/library.h?lines=1004%2C1012-1026 Contributors can now write their operator and register them within the same file using `REGISTER_OPERATORS` + `VK_REGISTER_OP()`, as shown in `Arithmetic.h/cpp`. Typically in Linux/Android C++ environments, the symbols corresponding to `OperatorRegisterInit` static instances are discarded since they aren't used for anything other than static initialization. Hence, we need to `link_whole = True` for the `vulkan_graph_runtime` library. We update our Compute API tests to verify we can go through `OperatorRegistry` with proper static initialization. ghstack-source-id: 217867140 @exported-using-ghexport Differential Revision: [D54641117](https://our.internmc.facebook.com/intern/diff/D54641117/)

This change follows 1. in the footsteps of #2222 for static initialization and 2. the popular `TorchLibraryImpl` for wrapping with macros. https://www.internalfb.com/code/fbsource/[b6860acf0fd7a95224f2ed3f6fe48f699a9a45c0]/fbcode/caffe2/torch/library.h?lines=1004%2C1012-1026 Contributors can now write their operator and register them within the same file using `REGISTER_OPERATORS` + `VK_REGISTER_OP()`, as shown in `Arithmetic.h/cpp`. Typically in Linux/Android C++ environments, the symbols corresponding to `OperatorRegisterInit` static instances are discarded since they aren't used for anything other than static initialization. Hence, we need to `link_whole = True` for the `vulkan_graph_runtime` library. We update our Compute API tests to verify we can go through `OperatorRegistry` with proper static initialization. Differential Revision: [D54641117](https://our.internmc.facebook.com/intern/diff/D54641117/) [ghstack-poisoned]

facebook-github-bot · 2024-03-07T23:43:13Z

This pull request was exported from Phabricator. Differential Revision: D54641117

Pull Request resolved: #2305 This change follows 1. in the footsteps of #2222 for static initialization and 2. the popular `TorchLibraryImpl` for wrapping with macros. https://www.internalfb.com/code/fbsource/[b6860acf0fd7a95224f2ed3f6fe48f699a9a45c0]/fbcode/caffe2/torch/library.h?lines=1004%2C1012-1026 Contributors can now write their operator and register them within the same file using `REGISTER_OPERATORS` + `VK_REGISTER_OP()`, as shown in `Arithmetic.h/cpp`. Typically in Linux/Android C++ environments, the symbols corresponding to `OperatorRegisterInit` static instances are discarded since they aren't used for anything other than static initialization. Hence, we need to `link_whole = True` for the `vulkan_graph_runtime` library. We update our Compute API tests to verify we can go through `OperatorRegistry` with proper static initialization. ghstack-source-id: 217871298 @exported-using-ghexport Differential Revision: [D54641117](https://our.internmc.facebook.com/intern/diff/D54641117/)

This must land before #2305. Otherwise, we will have two definitions of the same function signature in `executorch/backends/vulkan/runtime/graph/ops/OpUtils.cpp` and `ATen/native/vulkan/impl/Common.cpp`, resulting in a linker error, when building `vulkan_compute_api_test_bin`: ``` ld.lld: error: duplicate symbol: at::native::vulkan::adaptive_work_group_size(at::native::vulkan::api::utils::detail::vec<unsigned int, 3u> const&) >>> defined at Common.cpp:8 (./xplat/caffe2/aten/src/ATen/native/vulkan/impl/Common.cpp:8) >>> __objects__/aten/src/ATen/native/vulkan/impl/Common.cpp.o:(at::native::vulkan::adaptive_work_group_size(at::native::vulkan::api::utils::detail::vec<unsigned int, 3u> const&)) in archive buck-out/v2/gen/fbsource/a9b839d0bc77bff6/xplat/caffe2/__torch_vulkan_ops__/libtorch_vulkan_ops.a >>> defined at OpUtils.cpp:16 (./xplat/executorch/backends/vulkan/runtime/graph/ops/OpUtils.cpp:16) >>> __objects__/runtime/graph/ops/OpUtils.cpp.o:(.text._ZN2at6native6vulkan24adaptive_work_group_sizeERKNS1_3api5utils6detail3vecIjLj3EEE+0x0) in archive buck-out/v2/gen/fbsource/a9b839d0bc77bff6/xplat/executorch/backends/vulkan/__vulkan_graph_runtime__/libvulkan_graph_runtime.a clang-15: error: linker command failed with exit code 1 (use -v to see invocation) ``` These files are part of the `torch_vulkan_ops` and `vulkan_runtime_graph` libraries, respectively. We resolve the issue by removing the dependency of `vulkan_compute_api_test_bin` to on `torch_vulkan_ops`. Unfortunately, this requires copy-pasting more code to `VulkanBackend.cpp`. Differential Revision: [D54659269](https://our.internmc.facebook.com/intern/diff/D54659269/) [ghstack-poisoned]

This change follows 1. in the footsteps of #2222 for static initialization and 2. the popular `TorchLibraryImpl` for wrapping with macros. https://www.internalfb.com/code/fbsource/[b6860acf0fd7a95224f2ed3f6fe48f699a9a45c0]/fbcode/caffe2/torch/library.h?lines=1004%2C1012-1026 Contributors can now write their operator and register them within the same file using `REGISTER_OPERATORS` + `VK_REGISTER_OP()`, as shown in `Arithmetic.h/cpp`. Typically in Linux/Android C++ environments, the symbols corresponding to `OperatorRegisterInit` static instances are discarded since they aren't used for anything other than static initialization. Hence, we need to `link_whole = True` for the `vulkan_graph_runtime` library. We update our Compute API tests to verify we can go through `OperatorRegistry` with proper static initialization. Differential Revision: [D54641117](https://our.internmc.facebook.com/intern/diff/D54641117/) [ghstack-poisoned]

facebook-github-bot · 2024-03-08T01:09:48Z

This pull request was exported from Phabricator. Differential Revision: D54641117

Pull Request resolved: #2305 This change follows 1. in the footsteps of #2222 for static initialization and 2. the popular `TorchLibraryImpl` for wrapping with macros. https://www.internalfb.com/code/fbsource/[b6860acf0fd7a95224f2ed3f6fe48f699a9a45c0]/fbcode/caffe2/torch/library.h?lines=1004%2C1012-1026 Contributors can now write their operator and register them within the same file using `REGISTER_OPERATORS` + `VK_REGISTER_OP()`, as shown in `Arithmetic.h/cpp`. Typically in Linux/Android C++ environments, the symbols corresponding to `OperatorRegisterInit` static instances are discarded since they aren't used for anything other than static initialization. Hence, we need to `link_whole = True` for the `vulkan_graph_runtime` library. We update our Compute API tests to verify we can go through `OperatorRegistry` with proper static initialization. ghstack-source-id: 217884083 @exported-using-ghexport Differential Revision: [D54641117](https://our.internmc.facebook.com/intern/diff/D54641117/)

Summary: bypass-github-export-checks Pull Request resolved: #2312 This must land before #2305. Otherwise, we will have two definitions of the same function signature in `executorch/backends/vulkan/runtime/graph/ops/OpUtils.cpp` and `ATen/native/vulkan/impl/Common.cpp`, resulting in a linker error, when building `vulkan_compute_api_test_bin`: ``` ld.lld: error: duplicate symbol: at::native::vulkan::adaptive_work_group_size(at::native::vulkan::api::utils::detail::vec<unsigned int, 3u> const&) >>> defined at Common.cpp:8 (./xplat/caffe2/aten/src/ATen/native/vulkan/impl/Common.cpp:8) >>> __objects__/aten/src/ATen/native/vulkan/impl/Common.cpp.o:(at::native::vulkan::adaptive_work_group_size(at::native::vulkan::api::utils::detail::vec<unsigned int, 3u> const&)) in archive buck-out/v2/gen/fbsource/a9b839d0bc77bff6/xplat/caffe2/__torch_vulkan_ops__/libtorch_vulkan_ops.a >>> defined at OpUtils.cpp:16 (./xplat/executorch/backends/vulkan/runtime/graph/ops/OpUtils.cpp:16) >>> __objects__/runtime/graph/ops/OpUtils.cpp.o:(.text._ZN2at6native6vulkan24adaptive_work_group_sizeERKNS1_3api5utils6detail3vecIjLj3EEE+0x0) in archive buck-out/v2/gen/fbsource/a9b839d0bc77bff6/xplat/executorch/backends/vulkan/__vulkan_graph_runtime__/libvulkan_graph_runtime.a clang-15: error: linker command failed with exit code 1 (use -v to see invocation) ``` These files are part of the `torch_vulkan_ops` and `vulkan_runtime_graph` libraries, respectively. We resolve the issue by removing the dependency of `vulkan_compute_api_test_bin` to on `torch_vulkan_ops`. Unfortunately, this requires copy-pasting more code to `VulkanBackend.cpp`. ghstack-source-id: 217882620 exported-using-ghexport Reviewed By: SS-JIA Differential Revision: D54659269 fbshipit-source-id: 4d780c08d2a75b9e36ca1d646f5794a285b8086b

facebook-github-bot · 2024-03-08T03:48:21Z

This pull request has been merged in f3487ed.

In #2305, I added the `soname = "libvulkan_graph_runtime.$(ext)"` to the wrong library. Noob mistake. Also, removed an unneeded dependency. Differential Revision: [D54688153](https://our.internmc.facebook.com/intern/diff/D54688153/) [ghstack-poisoned]

In #2305, I added the `soname = "libvulkan_graph_runtime.$(ext)"` to the wrong library. Noob mistake. Also, removed an unneeded dependency. Differential Revision: [D54688153](https://our.internmc.facebook.com/intern/diff/D54688153/) ghstack-source-id: 217969105 Pull Request resolved: #2319

Summary: bypass-github-export-checks Pull Request resolved: #2319 In #2305, I added the `soname = "libvulkan_graph_runtime.$(ext)"` to the wrong library. Noob mistake. Also, I'd forgotten to remove an unneeded dependency. Reviewed By: SS-JIA Differential Revision: D54688153 fbshipit-source-id: 67a1316c76180030d4ae5225f16f01dcc750220a

facebook-github-bot added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label Mar 7, 2024

jorgep31415 mentioned this pull request Mar 7, 2024

[ET-VK][EZ] Clean up OperatorRegistry #2304

Closed

facebook-github-bot added the fb-exported label Mar 7, 2024

SS-JIA approved these changes Mar 7, 2024

View reviewed changes

jorgep31415 mentioned this pull request Mar 8, 2024

[ET-VK] Remove test dependency on ATen/native/vulkan/impl #2312

Closed

facebook-github-bot closed this in f3487ed Mar 8, 2024

facebook-github-bot added the Merged label Mar 8, 2024

jorgep31415 mentioned this pull request Mar 8, 2024

[ET-VK][EZ] Fix Android test linkage #2319

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

[ET-VK] Enable dynamic operator registration #2305

[ET-VK] Enable dynamic operator registration #2305

Uh oh!

jorgep31415 commented Mar 7, 2024 •

edited

Loading

Uh oh!

pytorch-bot bot commented Mar 7, 2024 •

edited

Loading

Uh oh!

facebook-github-bot commented Mar 7, 2024

Uh oh!

facebook-github-bot commented Mar 7, 2024

Uh oh!

facebook-github-bot commented Mar 7, 2024

Uh oh!

facebook-github-bot commented Mar 8, 2024

Uh oh!

facebook-github-bot commented Mar 8, 2024

Uh oh!

Uh oh!

[ET-VK] Enable dynamic operator registration #2305

[ET-VK] Enable dynamic operator registration #2305

Uh oh!

Conversation

jorgep31415 commented Mar 7, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

pytorch-bot bot commented Mar 7, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

🔗 Helpful Links

🧪 See artifacts and rendered test results at hud.pytorch.org/pr/pytorch/executorch/2305

❌ 1 New Failure, 1 Unrelated Failure

Uh oh!

facebook-github-bot commented Mar 7, 2024

Uh oh!

facebook-github-bot commented Mar 7, 2024

Uh oh!

facebook-github-bot commented Mar 7, 2024

Uh oh!

facebook-github-bot commented Mar 8, 2024

Uh oh!

facebook-github-bot commented Mar 8, 2024

Uh oh!

Uh oh!

jorgep31415 commented Mar 7, 2024 •

edited

Loading

pytorch-bot bot commented Mar 7, 2024 •

edited

Loading