Skip to content
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

[Vulkan][Refactor] Pull out vulkan initialization into VulkanInstance and VulkanDevice #8188

Merged
merged 4 commits into from
Jun 4, 2021

Conversation

Lunderberg
Copy link
Contributor

Next step of the refactoring initiated in #8157. Each of the following changes is implemented in a separate commit in this PR.

  • Broke out VkInstance setup/teardown into managed class. Previously, the VkInstance was directly owned by the VulkanDeviceAPI. Now, VulkanDeviceAPI owns a tvm::runtime::vulkan::VulkanInstance that does setup/teardown of the VkInstance. This way, the teardown is done even if a later initialization step throws an exception.

  • Renamed VulkanContext to VulkanDevice, to match with the tvm.context to tvm.device rename.

  • Extracted VulkanDevice initialization into VulkanDevice class

  • Removed the VkPhysicalDeviceProperties member variable from VulkanDevice. With the separate VulkanDeviceProperties class, this moves all device parameter query/access to a single path.

…lass.

- Previously, the VkInstance was directly owned by the
  VulkanDeviceAPI.  Now, VulkanDeviceAPI owns a
  tvm::runtime::vulkan::VulkanInstance that does setup/teardown of the
  VkInstance.  This way, the teardown is done even if a later
  initialization step throws an exception.
Renaming to match with the tvm.context to tvm.device rename.
…able from VulkanDevice

- Now that there is a separate VulkanDeviceProperties class, the
  redundant VkPhysicalDeviceProperties can be removed.
@Lunderberg
Copy link
Contributor Author

Potential reviewers: @masahi @tmoreau89

@masahi masahi merged commit ae4a3be into apache:main Jun 4, 2021
@masahi
Copy link
Member

masahi commented Jun 4, 2021

thanks @Lunderberg

@Lunderberg Lunderberg deleted the vulkan_refactor_part1 branch June 4, 2021 15:02
trevor-m pushed a commit to trevor-m/tvm that referenced this pull request Jun 17, 2021
… and VulkanDevice (apache#8188)

* [Vulkan][Refactor] Broke out VkInstance setup/teardown into managed class.

- Previously, the VkInstance was directly owned by the
  VulkanDeviceAPI.  Now, VulkanDeviceAPI owns a
  tvm::runtime::vulkan::VulkanInstance that does setup/teardown of the
  VkInstance.  This way, the teardown is done even if a later
  initialization step throws an exception.

* [Vulkan] Renamed VulkanContext to VulkanDevice

Renaming to match with the tvm.context to tvm.device rename.

* [Vulkan][Refactor] Extracted VulkanDevice initialization into VulkanDevice class

* [Vulkan][Refactor] Removed the VkPhysicalDeviceProperties member variable from VulkanDevice

- Now that there is a separate VulkanDeviceProperties class, the
  redundant VkPhysicalDeviceProperties can be removed.

Co-authored-by: Eric Lunderberg <elunderberg@octoml.ai>
trevor-m pushed a commit to neo-ai/tvm that referenced this pull request Jun 17, 2021
… and VulkanDevice (apache#8188)

* [Vulkan][Refactor] Broke out VkInstance setup/teardown into managed class.

- Previously, the VkInstance was directly owned by the
  VulkanDeviceAPI.  Now, VulkanDeviceAPI owns a
  tvm::runtime::vulkan::VulkanInstance that does setup/teardown of the
  VkInstance.  This way, the teardown is done even if a later
  initialization step throws an exception.

* [Vulkan] Renamed VulkanContext to VulkanDevice

Renaming to match with the tvm.context to tvm.device rename.

* [Vulkan][Refactor] Extracted VulkanDevice initialization into VulkanDevice class

* [Vulkan][Refactor] Removed the VkPhysicalDeviceProperties member variable from VulkanDevice

- Now that there is a separate VulkanDeviceProperties class, the
  redundant VkPhysicalDeviceProperties can be removed.

Co-authored-by: Eric Lunderberg <elunderberg@octoml.ai>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants