[vulkan] Ops registration to TORCH_LIBRARY_IMPL#42194
Closed
IvanKobzarev wants to merge 7 commits intogh/IvanKobzarev/65/basefrom
Closed
[vulkan] Ops registration to TORCH_LIBRARY_IMPL#42194IvanKobzarev wants to merge 7 commits intogh/IvanKobzarev/65/basefrom
IvanKobzarev wants to merge 7 commits intogh/IvanKobzarev/65/basefrom
Conversation
[ghstack-poisoned]
💊 CI failures summary and remediationsAs of commit e467951 (more details on the Dr. CI page): 💚 💚 Looks good so far! There are no failures yet. 💚 💚 This comment was automatically generated by Dr. CI (expand for details).Follow this link to opt-out of these comments for your Pull Requests.Please report bugs/suggestions on the GitHub issue tracker or post in the (internal) Dr. CI Users group. This comment has been revised 22 times. |
Differential Revision: [D22803036](https://our.internmc.facebook.com/intern/diff/D22803036) The main intention for this change is for the Vulkan Backend buck build. Where the Vulkan part can be optionally linked to general ATen compiled _without_ USE_VULKAN flag. For this, we have to remove all preprocessor logic based on `#ifdef USE_VULKAN`. These includes: - Registering Vulkan operators through TORCH_LIBRARY_IMPL and removing them from `native_functions.yaml` - Removing `aten/src/ATen/native/vulkan/stub/VulkanAtenStub.cpp` as it registered stubs for vulkan functions without linked Vulkan backend; Changing CMakeLists.txt associated with it - Removing preprocessor logic for incode dispatching to vulkan ops in `aten/src/ATen/native/Convolution.cpp`, `aten/src/ATen/native/AdaptiveAveragePooling.cpp`, `aten/src/ATen/native/Copy.cpp` - Vulkan ops in VulkanAten.cpp without registration in native_functions.yaml can be moved from namespace at::native to more specific at::native::vulkan::aten; Ops are registered in this file with TORCH_LIBRARY_IMPL. - copy is dispatched as CatchAll. As we do not have preprocessor flag USE_VULKAN, we need to have their code for logic if src or dst tensor is on Vulkan backend. The same for function `is_vulkan_available()` that we need for test guards and check if vulkan backend initialized successfully - it should be presented with and without Vulkan backend linked. So now it has `if` condition without preprocessor, that checks if src or dst is Vulkan then calls vulkan_copy from `aten/src/ATen/vulkan/Context.cpp`. `aten/src/ATen/vulkan/Context.cpp` contains runtime registration of `VulkanImplInterface*` and dispatches `vulkan_copy_` and `is_vulkan_available` to no-op if aten was not linked with Vulkan backend and to real implementation if it was linked with it. (Registration is placed in `VulkanAten.cpp`: ``` static at::vulkan::VulkanImplRegistrar g_vulkan_impl(new VulkanImpl()); ``` [ghstack-poisoned]
Differential Revision: [D22803036](https://our.internmc.facebook.com/intern/diff/D22803036) The main intention for this change is for the Vulkan Backend buck build. Where the Vulkan part can be optionally linked to general ATen compiled _without_ USE_VULKAN flag. For this, we have to remove all preprocessor logic based on `#ifdef USE_VULKAN`. These includes: - Registering Vulkan operators through TORCH_LIBRARY_IMPL and removing them from `native_functions.yaml` - Removing `aten/src/ATen/native/vulkan/stub/VulkanAtenStub.cpp` as it registered stubs for vulkan functions without linked Vulkan backend; Changing CMakeLists.txt associated with it - Removing preprocessor logic for incode dispatching to vulkan ops in `aten/src/ATen/native/Convolution.cpp`, `aten/src/ATen/native/AdaptiveAveragePooling.cpp`, `aten/src/ATen/native/Copy.cpp` - Vulkan ops in VulkanAten.cpp without registration in native_functions.yaml can be moved from namespace at::native to more specific at::native::vulkan::aten; Ops are registered in this file with TORCH_LIBRARY_IMPL. - copy is dispatched as CatchAll. As we do not have preprocessor flag USE_VULKAN, we need to have their code for logic if src or dst tensor is on Vulkan backend. The same for function `is_vulkan_available()` that we need for test guards and check if vulkan backend initialized successfully - it should be presented with and without Vulkan backend linked. So now it has `if` condition without preprocessor, that checks if src or dst is Vulkan then calls vulkan_copy from `aten/src/ATen/vulkan/Context.cpp`. `aten/src/ATen/vulkan/Context.cpp` contains runtime registration of `VulkanImplInterface*` and dispatches `vulkan_copy_` and `is_vulkan_available` to no-op if aten was not linked with Vulkan backend and to real implementation if it was linked with it. (Registration is placed in `VulkanAten.cpp`: ``` static at::vulkan::VulkanImplRegistrar g_vulkan_impl(new VulkanImpl()); ``` [ghstack-poisoned]
Differential Revision: [D22803036](https://our.internmc.facebook.com/intern/diff/D22803036) The main intention for this change is for the Vulkan Backend buck build. Where the Vulkan part can be optionally linked to general ATen compiled _without_ USE_VULKAN flag. For this, we have to remove all preprocessor logic based on `#ifdef USE_VULKAN`. These includes: - Registering Vulkan operators through TORCH_LIBRARY_IMPL and removing them from `native_functions.yaml` - Removing `aten/src/ATen/native/vulkan/stub/VulkanAtenStub.cpp` as it registered stubs for vulkan functions without linked Vulkan backend; Changing CMakeLists.txt associated with it - Removing preprocessor logic for incode dispatching to vulkan ops in `aten/src/ATen/native/Convolution.cpp`, `aten/src/ATen/native/AdaptiveAveragePooling.cpp`, `aten/src/ATen/native/Copy.cpp` - Vulkan ops in VulkanAten.cpp without registration in native_functions.yaml can be moved from namespace at::native to more specific at::native::vulkan::aten; Ops are registered in this file with TORCH_LIBRARY_IMPL. - copy is dispatched as CatchAll. As we do not have preprocessor flag USE_VULKAN, we need to have their code for logic if src or dst tensor is on Vulkan backend. The same for function `is_vulkan_available()` that we need for test guards and check if vulkan backend initialized successfully - it should be presented with and without Vulkan backend linked. So now it has `if` condition without preprocessor, that checks if src or dst is Vulkan then calls vulkan_copy from `aten/src/ATen/vulkan/Context.cpp`. `aten/src/ATen/vulkan/Context.cpp` contains runtime registration of `VulkanImplInterface*` and dispatches `vulkan_copy_` and `is_vulkan_available` to no-op if aten was not linked with Vulkan backend and to real implementation if it was linked with it. (Registration is placed in `VulkanAten.cpp`: ``` static at::vulkan::VulkanImplRegistrar g_vulkan_impl(new VulkanImpl()); ``` [ghstack-poisoned]
Differential Revision: [D22803036](https://our.internmc.facebook.com/intern/diff/D22803036) The main intention for this change is for the Vulkan Backend buck build. Where the Vulkan part can be optionally linked to general ATen compiled _without_ USE_VULKAN flag. For this, we have to remove all preprocessor logic based on `#ifdef USE_VULKAN`. These includes: - Registering Vulkan operators through TORCH_LIBRARY_IMPL and removing them from `native_functions.yaml` - Removing `aten/src/ATen/native/vulkan/stub/VulkanAtenStub.cpp` as it registered stubs for vulkan functions without linked Vulkan backend; Changing CMakeLists.txt associated with it - Removing preprocessor logic for incode dispatching to vulkan ops in `aten/src/ATen/native/Convolution.cpp`, `aten/src/ATen/native/AdaptiveAveragePooling.cpp`, `aten/src/ATen/native/Copy.cpp` - Vulkan ops in VulkanAten.cpp without registration in native_functions.yaml can be moved from namespace at::native to more specific at::native::vulkan::aten; Ops are registered in this file with TORCH_LIBRARY_IMPL. - copy is dispatched as CatchAll. As we do not have preprocessor flag USE_VULKAN, we need to have their code for logic if src or dst tensor is on Vulkan backend. The same for function `is_vulkan_available()` that we need for test guards and check if vulkan backend initialized successfully - it should be presented with and without Vulkan backend linked. So now it has `if` condition without preprocessor, that checks if src or dst is Vulkan then calls vulkan_copy from `aten/src/ATen/vulkan/Context.cpp`. `aten/src/ATen/vulkan/Context.cpp` contains runtime registration of `VulkanImplInterface*` and dispatches `vulkan_copy_` and `is_vulkan_available` to no-op if aten was not linked with Vulkan backend and to real implementation if it was linked with it. (Registration is placed in `VulkanAten.cpp`: ``` static at::vulkan::VulkanImplRegistrar g_vulkan_impl(new VulkanImpl()); ``` [ghstack-poisoned]
AshkanAliabadi
approved these changes
Aug 6, 2020
Differential Revision: [D22803036](https://our.internmc.facebook.com/intern/diff/D22803036) The main intention for this change is for the Vulkan Backend buck build. Where the Vulkan part can be optionally linked to general ATen compiled _without_ USE_VULKAN flag. For this, we have to remove all preprocessor logic based on `#ifdef USE_VULKAN`. These includes: - Registering Vulkan operators through TORCH_LIBRARY_IMPL and removing them from `native_functions.yaml` - Removing `aten/src/ATen/native/vulkan/stub/VulkanAtenStub.cpp` as it registered stubs for vulkan functions without linked Vulkan backend; Changing CMakeLists.txt associated with it - Removing preprocessor logic for incode dispatching to vulkan ops in `aten/src/ATen/native/Convolution.cpp`, `aten/src/ATen/native/AdaptiveAveragePooling.cpp`, `aten/src/ATen/native/Copy.cpp` - Vulkan ops in VulkanAten.cpp without registration in native_functions.yaml can be moved from namespace at::native to more specific at::native::vulkan::aten; Ops are registered in this file with TORCH_LIBRARY_IMPL. - copy is dispatched as CatchAll. As we do not have preprocessor flag USE_VULKAN, we need to have their code for logic if src or dst tensor is on Vulkan backend. The same for function `is_vulkan_available()` that we need for test guards and check if vulkan backend initialized successfully - it should be presented with and without Vulkan backend linked. So now it has `if` condition without preprocessor, that checks if src or dst is Vulkan then calls vulkan_copy from `aten/src/ATen/vulkan/Context.cpp`. `aten/src/ATen/vulkan/Context.cpp` contains runtime registration of `VulkanImplInterface*` and dispatches `vulkan_copy_` and `is_vulkan_available` to no-op if aten was not linked with Vulkan backend and to real implementation if it was linked with it. (Registration is placed in `VulkanAten.cpp`: ``` static at::vulkan::VulkanImplRegistrar g_vulkan_impl(new VulkanImpl()); ``` [ghstack-poisoned]
Differential Revision: [D22803036](https://our.internmc.facebook.com/intern/diff/D22803036) The main intention for this change is for the Vulkan Backend buck build. Where the Vulkan part can be optionally linked to general ATen compiled _without_ USE_VULKAN flag. For this, we have to remove all preprocessor logic based on `#ifdef USE_VULKAN`. These includes: - Registering Vulkan operators through TORCH_LIBRARY_IMPL and removing them from `native_functions.yaml` - Removing `aten/src/ATen/native/vulkan/stub/VulkanAtenStub.cpp` as it registered stubs for vulkan functions without linked Vulkan backend; Changing CMakeLists.txt associated with it - Removing preprocessor logic for incode dispatching to vulkan ops in `aten/src/ATen/native/Convolution.cpp`, `aten/src/ATen/native/AdaptiveAveragePooling.cpp`, `aten/src/ATen/native/Copy.cpp` - Vulkan ops in VulkanAten.cpp without registration in native_functions.yaml can be moved from namespace at::native to more specific at::native::vulkan::aten; Ops are registered in this file with TORCH_LIBRARY_IMPL. - copy is dispatched as CatchAll. As we do not have preprocessor flag USE_VULKAN, we need to have their code for logic if src or dst tensor is on Vulkan backend. The same for function `is_vulkan_available()` that we need for test guards and check if vulkan backend initialized successfully - it should be presented with and without Vulkan backend linked. So now it has `if` condition without preprocessor, that checks if src or dst is Vulkan then calls vulkan_copy from `aten/src/ATen/vulkan/Context.cpp`. `aten/src/ATen/vulkan/Context.cpp` contains runtime registration of `VulkanImplInterface*` and dispatches `vulkan_copy_` and `is_vulkan_available` to no-op if aten was not linked with Vulkan backend and to real implementation if it was linked with it. (Registration is placed in `VulkanAten.cpp`: ``` static at::vulkan::VulkanImplRegistrar g_vulkan_impl(new VulkanImpl()); ``` [ghstack-poisoned]
Contributor
|
@IvanKobzarev merged this pull request in 3c66a37. |
Contributor
|
@IvanKobzarev I'm super late to the party here, but I just want to point out that one other consequence of moving things out of |
ezyang
added a commit
that referenced
this pull request
Oct 27, 2020
It turns out that after #42194 landed we no longer actually generate any registrations into this file. That means it's completely unnecessary. Signed-off-by: Edward Z. Yang <ezyang@fb.com> [ghstack-poisoned]
ezyang
added a commit
that referenced
this pull request
Oct 27, 2020
It turns out that after #42194 landed we no longer actually generate any registrations into this file. That means it's completely unnecessary. Signed-off-by: Edward Z. Yang <ezyang@fb.com> Differential Revision: [D24573518](https://our.internmc.facebook.com/intern/diff/D24573518) [ghstack-poisoned]
ezyang
added a commit
that referenced
this pull request
Oct 28, 2020
It turns out that after #42194 landed we no longer actually generate any registrations into this file. That means it's completely unnecessary. Signed-off-by: Edward Z. Yang <ezyang@fb.com> Differential Revision: [D24573518](https://our.internmc.facebook.com/intern/diff/D24573518) [ghstack-poisoned]
ezyang
added a commit
that referenced
this pull request
Oct 28, 2020
It turns out that after #42194 landed we no longer actually generate any registrations into this file. That means it's completely unnecessary. Signed-off-by: Edward Z. Yang <ezyang@fb.com> Differential Revision: [D24573518](https://our.internmc.facebook.com/intern/diff/D24573518) [ghstack-poisoned]
facebook-github-bot
pushed a commit
that referenced
this pull request
Oct 29, 2020
Summary: Pull Request resolved: #46938 It turns out that after #42194 landed we no longer actually generate any registrations into this file. That means it's completely unnecessary. Signed-off-by: Edward Z. Yang <ezyang@fb.com> Test Plan: Imported from OSS Reviewed By: IvanKobzarev Differential Revision: D24573518 Pulled By: ezyang fbshipit-source-id: b41ada9e394b780f037f5977596a36b896b5648c
laurentdupin
pushed a commit
to laurentdupin/pytorch
that referenced
this pull request
Apr 24, 2026
Summary: Pull Request resolved: pytorch#42194 Test Plan: Imported from OSS Reviewed By: AshkanAliabadi Differential Revision: D22803036 Pulled By: IvanKobzarev fbshipit-source-id: 2f402541aecf887d78f650bf05d758a0e403bc4d
laurentdupin
pushed a commit
to laurentdupin/pytorch
that referenced
this pull request
Apr 24, 2026
Summary: Pull Request resolved: pytorch#46938 It turns out that after pytorch#42194 landed we no longer actually generate any registrations into this file. That means it's completely unnecessary. Signed-off-by: Edward Z. Yang <ezyang@fb.com> Test Plan: Imported from OSS Reviewed By: IvanKobzarev Differential Revision: D24573518 Pulled By: ezyang fbshipit-source-id: b41ada9e394b780f037f5977596a36b896b5648c
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Stack from ghstack:
Differential Revision: D22803036
The main intention for this change is for the Vulkan Backend buck build.
Where the Vulkan part can be optionally linked to general ATen compiled without USE_VULKAN flag.
For this, we have to remove all preprocessor logic based on
#ifdef USE_VULKAN.These includes:
Registering Vulkan operators through TORCH_LIBRARY_IMPL and removing them from
native_functions.yamlRemoving
aten/src/ATen/native/vulkan/stub/VulkanAtenStub.cppas it registered stubs for vulkan functions without linked Vulkan backend; Changing CMakeLists.txt associated with itRemoving preprocessor logic for incode dispatching to vulkan ops in
aten/src/ATen/native/Convolution.cpp,aten/src/ATen/native/AdaptiveAveragePooling.cpp,aten/src/ATen/native/Copy.cppVulkan ops in VulkanAten.cpp without registration in native_functions.yaml can be moved from namespace at::native to more specific at::native::vulkan::aten; Ops are registered in this file with TORCH_LIBRARY_IMPL.
copy is dispatched as CatchAll. As we do not have preprocessor flag USE_VULKAN, we need to have their code for logic if src or dst tensor is on Vulkan backend.
The same for function
is_vulkan_available()that we need for test guards and check if vulkan backend initialized successfully - it should be presented with and without Vulkan backend linked.So now it has
ifcondition without preprocessor, that checks if src or dst is Vulkan then calls vulkan_copy fromaten/src/ATen/vulkan/Context.cpp.aten/src/ATen/vulkan/Context.cppcontains runtime registration ofVulkanImplInterface*and dispatchesvulkan_copy_andis_vulkan_availableto no-op if aten was not linked with Vulkan backend and to real implementation if it was linked with it. (Registration is placed inVulkanAten.cpp: