Skip to content

[ET-VK] Replacing the use of uvec3 with WorkgroupSize class to reduce memory usage and improve processing speed #8634

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

Merged
merged 5 commits into from
Feb 25, 2025

Conversation

trivedivivek
Copy link
Contributor

@trivedivivek trivedivivek commented Feb 22, 2025

Stack from ghstack (oldest at bottom):

This diff replaces the use of uvec3 with WorkgroupSize class to reduce memory usage and improve processing speed in the Vulkan backend of Executorch.

Differential Revision: D70021032

… memory usage and improve processing speed

This diff replaces the use of `uvec3` with `WorkgroupSize` class to reduce memory usage and improve processing speed in the Vulkan backend of Executorch.

Differential Revision: [D70021032](https://our.internmc.facebook.com/intern/diff/D70021032/)

[ghstack-poisoned]
@trivedivivek trivedivivek requested a review from SS-JIA as a code owner February 22, 2025 05:35
Copy link

pytorch-bot bot commented Feb 22, 2025

🔗 Helpful Links

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

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

✅ No Failures

As of commit ba10936 with merge base 728c255 (image):
💚 Looks good so far! There are no failures yet. 💚

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

@facebook-github-bot 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 Feb 22, 2025
@facebook-github-bot
Copy link
Contributor

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

trivedivivek added a commit that referenced this pull request Feb 22, 2025
… memory usage and improve processing speed

This diff replaces the use of `uvec3` with `WorkgroupSize` class to reduce memory usage and improve processing speed in the Vulkan backend of Executorch.

Differential Revision: [D70021032](https://our.internmc.facebook.com/intern/diff/D70021032/)

ghstack-source-id: 267791087
Pull Request resolved: #8634
…s to reduce memory usage and improve processing speed"

This diff replaces the use of `uvec3` with `WorkgroupSize` class to reduce memory usage and improve processing speed in the Vulkan backend of Executorch.

Differential Revision: [D70021032](https://our.internmc.facebook.com/intern/diff/D70021032/)

[ghstack-poisoned]
@facebook-github-bot
Copy link
Contributor

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

trivedivivek added a commit that referenced this pull request Feb 22, 2025
… memory usage and improve processing speed

Pull Request resolved: #8634

This diff replaces the use of `uvec3` with `WorkgroupSize` class to reduce memory usage and improve processing speed in the Vulkan backend of Executorch.
ghstack-source-id: 267792195
@exported-using-ghexport

Differential Revision: [D70021032](https://our.internmc.facebook.com/intern/diff/D70021032/)
@trivedivivek trivedivivek changed the title [EK-VT] Replacing the use of uvec3 with WorkgroupSize class to reduce memory usage and improve processing speed [ET-VK] Replacing the use of uvec3 with WorkgroupSize class to reduce memory usage and improve processing speed Feb 24, 2025
…s to reduce memory usage and improve processing speed"

This diff replaces the use of `uvec3` with `WorkgroupSize` class to reduce memory usage and improve processing speed in the Vulkan backend of Executorch.

Differential Revision: [D70021032](https://our.internmc.facebook.com/intern/diff/D70021032/)

[ghstack-poisoned]
@facebook-github-bot
Copy link
Contributor

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

…s to reduce memory usage and improve processing speed"

This diff replaces the use of `uvec3` with `WorkgroupSize` class to reduce memory usage and improve processing speed in the Vulkan backend of Executorch.

Differential Revision: [D70021032](https://our.internmc.facebook.com/intern/diff/D70021032/)

[ghstack-poisoned]
@facebook-github-bot
Copy link
Contributor

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

…s to reduce memory usage and improve processing speed"

This diff replaces the use of `uvec3` with `WorkgroupSize` class to reduce memory usage and improve processing speed in the Vulkan backend of Executorch.

Differential Revision: [D70021032](https://our.internmc.facebook.com/intern/diff/D70021032/)

[ghstack-poisoned]
@facebook-github-bot
Copy link
Contributor

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

trivedivivek added a commit that referenced this pull request Feb 25, 2025
… memory usage and improve processing speed

Pull Request resolved: #8634

This diff replaces the use of `uvec3` with `WorkgroupSize` class to reduce memory usage and improve processing speed in the Vulkan backend of Executorch.
ghstack-source-id: 268172660
@exported-using-ghexport

Differential Revision: [D70021032](https://our.internmc.facebook.com/intern/diff/D70021032/)
@facebook-github-bot facebook-github-bot merged commit d9451ca into gh/trivedivivek/56/base Feb 25, 2025
50 checks passed
@facebook-github-bot facebook-github-bot deleted the gh/trivedivivek/56/head branch February 25, 2025 05:49
trivedivivek added a commit that referenced this pull request Feb 25, 2025
… memory usage and improve processing speed (#8671)

* [ET-VK] Adding a workgroup class to VecUtils

Pull Request resolved: #8632

This diff adds a new class called `WorkgroupSize` to the `VecUtils` header file. The `WorkgroupSize` class takes three `uint32_t` values as parameters and stores them in a single `uint32_t` variable using bitwise operations. This class is used in the Vulkan backend to specify the size of a workgroup for a given operation.
ghstack-source-id: 268172661
@exported-using-ghexport

Differential Revision: [D70021019](https://our.internmc.facebook.com/intern/diff/D70021019/)

* [ET-VK] Adding reserve and append functions to SpecVarList

Pull Request resolved: #8633

This diff adds two new functions to the SpecVarList class in the Vulkan runtime library. The first function, reserve, allows the user to reserve a certain amount of space in the SpecVarList before adding any elements. The second function, append, allows the user to add a single SpecVar to the SpecVarList. These functions are useful for optimizing memory usage and improving performance in the Vulkan runtime.
ghstack-source-id: 268172659
@exported-using-ghexport

Differential Revision: [D70021782](https://our.internmc.facebook.com/intern/diff/D70021782/)

* [ET-VK] Replacing the use of uvec3 with WorkgroupSize class to reduce memory usage and improve processing speed

Pull Request resolved: #8634

This diff replaces the use of `uvec3` with `WorkgroupSize` class to reduce memory usage and improve processing speed in the Vulkan backend of Executorch.
ghstack-source-id: 268172660
@exported-using-ghexport

Differential Revision: [D70021032](https://our.internmc.facebook.com/intern/diff/D70021032/)

---------

Co-authored-by: Vivek Trivedi <[email protected]>
SS-JIA added a commit that referenced this pull request Mar 19, 2025
…line`

## Context

Fix changes introduced in #8634 (D70021032).

This change decoupled local work group size from additional specialization constants. As part of this change, when creating a `VkComputePipeline` a temporary `SpecVarList` is created to merge the `WorkgroupSize` with additional specialization constants.

However, this can be an issue with some Vulkan drivers because the `SpecVarList` is a temporary, and thus will be destroyed at the end of the function call. The pointer stored in the `VkSpecializationInfo` will be invalidated, leading to undefined behaviour.


This diff fixes this by restoring the behaviour of `ComputePipeline::Descriptor` storing the `specialization_constants` directly.

Also fix the fact that the `VkSpecializationMapEntry` vector was also a temporary when creating a `VkComputePipeline` by storing it in `ComputePipeline`.

Differential Revision: [D71488015](https://our.internmc.facebook.com/intern/diff/D71488015/)

[ghstack-poisoned]
SS-JIA added a commit that referenced this pull request Mar 19, 2025
…line`

## Context

Fix changes introduced in #8634 (D70021032).

This change decoupled local work group size from additional specialization constants. As part of this change, when creating a `VkComputePipeline` a temporary `SpecVarList` is created to merge the `WorkgroupSize` with additional specialization constants.

However, this can be an issue with some Vulkan drivers because the `SpecVarList` is a temporary, and thus will be destroyed at the end of the function call. The pointer stored in the `VkSpecializationInfo` will be invalidated, leading to undefined behaviour.


This diff fixes this by restoring the behaviour of `ComputePipeline::Descriptor` storing the `specialization_constants` directly.

Also fix the fact that the `VkSpecializationMapEntry` vector was also a temporary when creating a `VkComputePipeline` by storing it in `ComputePipeline`.

Differential Revision: [D71488015](https://our.internmc.facebook.com/intern/diff/D71488015/)

ghstack-source-id: 272783852
Pull Request resolved: #9405
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. fb-exported topic: not user facing
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants