[vulkan] Ops registration to TORCH_LIBRARY_IMPL #42194
Closed
Conversation
This file contains 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
[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]
IvanKobzarev
added a commit
that referenced
this pull request
Jul 28, 2020
ghstack-source-id: 25670a6370336d01b340fdd4c502be7c55b72ebe Pull Request resolved: #42194
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]
|
@IvanKobzarev merged this pull request in 3c66a37. |
|
@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
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: