Skip to content

[vulkan] Ops registration to TORCH_LIBRARY_IMPL#42194

Closed
IvanKobzarev wants to merge 7 commits intogh/IvanKobzarev/65/basefrom
gh/IvanKobzarev/65/head
Closed

[vulkan] Ops registration to TORCH_LIBRARY_IMPL#42194
IvanKobzarev wants to merge 7 commits intogh/IvanKobzarev/65/basefrom
gh/IvanKobzarev/65/head

Conversation

@IvanKobzarev
Copy link
Copy Markdown
Contributor

@IvanKobzarev IvanKobzarev commented Jul 28, 2020

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.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());

@dr-ci
Copy link
Copy Markdown

dr-ci Bot commented Jul 28, 2020

💊 CI failures summary and remediations

As 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.

See how this bot performed.

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: 25670a6
Pull Request resolved: #42194
@IvanKobzarev IvanKobzarev requested review from ljk53 and smessmer July 30, 2020 03:28
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]
@facebook-github-bot
Copy link
Copy Markdown
Contributor

@IvanKobzarev merged this pull request in 3c66a37.

@facebook-github-bot facebook-github-bot deleted the gh/IvanKobzarev/65/head branch August 11, 2020 14:16
@ezyang
Copy link
Copy Markdown
Contributor

ezyang commented Oct 27, 2020

@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 native_functions.yaml and into manual registrations is that you no longer automatically get device guard code generation. I'm not sure if this actually matters for you (I don't know if Vulkan has a concept of devices), but something to be aware 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>

ghstack-source-id: f224373
Pull Request resolved: #46938
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 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-source-id: 773487e
Pull Request resolved: #46938
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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants