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

[RFC] Roadmap to a smaller, faster, and cleaner Unified Device API #6832

Open
1 of 11 tasks
bobcao3 opened this issue Dec 7, 2022 · 2 comments
Open
1 of 11 tasks

[RFC] Roadmap to a smaller, faster, and cleaner Unified Device API #6832

bobcao3 opened this issue Dec 7, 2022 · 2 comments
Assignees
Labels
doc Documentation related issues & PRs

Comments

@bobcao3
Copy link
Collaborator

bobcao3 commented Dec 7, 2022

The long term goal for the Unified Device API, or the RHI, is to be isolated into its own repo and maintained separately from Taichi. Where we will create its own unit tests and integration tests to improve robustness and maintain strong isolation boundary between them.

In the end, we want to achieve these goals for the future RHI:

  1. RHI can be trimmed down through discarding unused modules. We should make RHI per-backend dynamically linked and optionally statically linked.
  2. RHI contains its own full functional specification and tests that uphold these specifications
  3. Provide a zero-overhead C-API
  4. Do not leak symbols to linked libraries & support different drivers and external dependencies robustly
  5. Able to use shared library if the client of RHI already uses them
  6. Being a fully functioned RHI that we can build other non-taichi applications on top of it
  7. Make it smaller in size and complexity

To achieve this goal while not hugely breaking the workflow and goals within Taichi, we will start by progressively trimming away dependencies of RHI. This hopefully will eventually lead to a clean separation when we move RHI outside of Taichi repo. Here are the first steps:

  • Remove all Taichi IR and related dependencies (ir.h, program.h, LLVM, eigen, etc.)
    • GFX
    • LLVM
  • Remove / avoid TI_XXX and spdlog based logging when required (use MACRO, log less, and use what's provided by standard library)
    • GFX
    • LLVM
  • Remove exceptions (use noexcept) and use clear status return values. (This reduces generated code size and helps with C-API impl)
    • GFX
    • LLVM
  • Re-organize the API to make most-of-the-time optional features being binary-wise optional. (e.g. Device/Context creation, Window/Surface creation, etc.)
  • Re-organize the binding model to help with integration into 3rd party code bases
@bobcao3 bobcao3 added the doc Documentation related issues & PRs label Dec 7, 2022
@bobcao3 bobcao3 self-assigned this Dec 7, 2022
@bobcao3
Copy link
Collaborator Author

bobcao3 commented Dec 11, 2022

Here to track the progress on the API surface (adding return status, adding documentation & specs, removing STL, or other changes):

  • ShaderResourceSet
    • Split into shader resource set and raster-state set
    • rw_buffer
    • buffer
    • image
    • rw_image
  • RasterResources
    • vertex_buffer
    • index_buffer
  • Pipeline
  • CommandList
    • bind_pipeline
    • bind_resources
    • buffer_barrier
    • memory_barrier
    • buffer_copy
    • buffer_fill
    • dispatch
    • begin_renderpass
    • end_renderpass
    • draw
    • draw_instance
    • set_line_width
    • draw_indexed
    • draw_indexed_instance
    • image_transition
    • buffer_to_image
    • image_to_buffer
    • copy_image
    • blit_image
    • signal_event
    • reset_event
    • wait_event
  • Stream
    • new_command_list
    • submit
    • submit_synced
    • command_sync
    • device_time_elapsed_us Redesign?
  • Device
    • allocate_memory
    • dealloc_memory
    • get_memory_physical_pointer
    • create_pipeline
    • create_event
    • allocate_memory_unique
    • fetch_result_uint64 Remove
    • get_compute_stream
    • wait_idle
    • map_range
    • map
    • unmap
    • share_to Redesign?
    • memcpy_internal Move / Redesign?
    • check_memcpy_capability Move / Redesign?
    • memcpy_direct Move / Redesign?
    • memcpy_via_staging Move / Redesign?
    • memcpy_via_host Move / Redesign?
    • arch
    • get_caps
    • set_caps
  • Surface
    • acquire_next_image
    • get_target_image
    • present_image
    • get_size
    • get_image_count
    • image_format
    • resize
    • get_depth_data Move / Redesign
    • get_image_data Move / Redesign
  • GraphicsDevice (Merge with Device?)
    • create_raster_pipeline
    • get_graphics_stream
    • create_surface Redesign?
    • create_image
    • destroy_image
    • image_transition Remove?
    • buffer_to_image Move / Redesign
    • image_to_buffer Move / Redesign

bobcao3 added a commit that referenced this issue Dec 12, 2022
Related: #6832

1. Removed the TI_XXX logging from Vulkan RHI and prepare for total
removal of spdlog from RHI
2. Started wrapping internal Vulkan RHI functions with `RhiReturn<>` to
return a error status instead of throwing exception
3. Start to work on the `implementation API` vs `public API` separation

Updating AOT demo as required:
taichi-dev/taichi-aot-demo#98

Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
@bobcao3
Copy link
Collaborator Author

bobcao3 commented Dec 12, 2022

DX11 Impl tracker: #6879
Vulkan Impl tracker: #6880
OpenGL Impl tracker: #6881
Metal Impl tracker: #6882

bobcao3 added a commit that referenced this issue Dec 30, 2022
…rResourceSet & RasterResources (#6954)

Issue: #6832

### Brief Summary

ResourceBinder is not split into two structures, one controls binding of
shared-accessible resources, the other is dedicated to control binding
of rasterizer states (vertex buffers, etc.)

This makes the mapping onto Vulkan DescriptorSets easier, and cleans up
the implementation of resource binding all over the place. In addition,
these binding states are now no longer attached to the program and can
be thus pre-filled / pre-allocated to achieve lower overhead.

Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
feisuzhu pushed a commit to feisuzhu/taichi that referenced this issue Dec 31, 2022
…rResourceSet & RasterResources (taichi-dev#6954)

Issue: taichi-dev#6832

### Brief Summary

ResourceBinder is not split into two structures, one controls binding of
shared-accessible resources, the other is dedicated to control binding
of rasterizer states (vertex buffers, etc.)

This makes the mapping onto Vulkan DescriptorSets easier, and cleans up
the implementation of resource binding all over the place. In addition,
these binding states are now no longer attached to the program and can
be thus pre-filled / pre-allocated to achieve lower overhead.

Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
bobcao3 added a commit that referenced this issue Jan 4, 2023
Issue: #6832

### Brief Summary

Compute-only CommandList APIs other than dispatch has been changed to
use `noexcept`, and the behavior specifications has been added, and
relevant checks are now in place for Vulkan and partially for DX11.

Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
feisuzhu pushed a commit to feisuzhu/taichi that referenced this issue Jan 5, 2023
…rResourceSet & RasterResources (taichi-dev#6954)

Issue: taichi-dev#6832

### Brief Summary

ResourceBinder is not split into two structures, one controls binding of
shared-accessible resources, the other is dedicated to control binding
of rasterizer states (vertex buffers, etc.)

This makes the mapping onto Vulkan DescriptorSets easier, and cleans up
the implementation of resource binding all over the place. In addition,
these binding states are now no longer attached to the program and can
be thus pre-filled / pre-allocated to achieve lower overhead.

Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
feisuzhu pushed a commit to feisuzhu/taichi that referenced this issue Jan 5, 2023
)

Issue: taichi-dev#6832

### Brief Summary

Compute-only CommandList APIs other than dispatch has been changed to
use `noexcept`, and the behavior specifications has been added, and
relevant checks are now in place for Vulkan and partially for DX11.

Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
bobcao3 added a commit that referenced this issue Jan 5, 2023
Issue: #6832

Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
bobcao3 added a commit that referenced this issue Jan 6, 2023
Issue: #6832

Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
bobcao3 added a commit that referenced this issue Jan 12, 2023
…#7091)

Issue: #6832

### Brief Summary

The `create_pipeline` API has been updated to new standards.

In addition, we now added (optional) support for backend caches such as
`VkPipelineCache`. This makes running cached taichi programs even
faster.

Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
bobcao3 added a commit that referenced this issue Mar 12, 2023
Issue: #6832

### Brief Summary

Allocate memory now returns a result code and runtime should validate
whether memory allocation succeeded or not

---------

Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
lin-hitonami pushed a commit that referenced this issue Apr 21, 2023
Issue: #6832

### Brief Summary

<!--
copilot:summary
-->
### <samp>🤖 Generated by Copilot at 8f04f4a</samp>

Remove the `fetch_result_uint64` method from the `Device` interface and
its subclasses, as it was redundant and inefficient. Use direct access
to the result buffer instead of copying or returning a single value.

### Walkthrough

<!--
copilot:walkthrough
-->
### <samp>🤖 Generated by Copilot at 8f04f4a</samp>

* Remove `fetch_result_uint64` method from `Device` interface and
subclasses, as it is redundant and inefficient
([link](https://github.com/taichi-dev/taichi/pull/7851/files?diff=unified&w=0#diff-e0843d34a17298595cd26f5b47b1933a0d18c5c795b0c91b73ec8541a2cdd3f9L142-L148),
[link](https://github.com/taichi-dev/taichi/pull/7851/files?diff=unified&w=0#diff-a7b85199b3c0a668f23bf0b723c126009e774437499d555de255883c1d1f4648L89-L90),
[link](https://github.com/taichi-dev/taichi/pull/7851/files?diff=unified&w=0#diff-e49f3cb85e1e9d5e83043e56f4d5d69c371bf8f40b4b653e31fc24eaea7e30c7L153-L157),
[link](https://github.com/taichi-dev/taichi/pull/7851/files?diff=unified&w=0#diff-36789bee03bd10430f6b60242918f8518e49eec53a9024c7ef6043de304712b4L101-L102),
[link](https://github.com/taichi-dev/taichi/pull/7851/files?diff=unified&w=0#diff-7919f5d7e33aafc72f27ed93febc58a0ac77c220ae718bf78ef134dad3790654L182-L188),
[link](https://github.com/taichi-dev/taichi/pull/7851/files?diff=unified&w=0#diff-01cbaa214a08d98caa33977f217853d54c85d51c03e2dab610909f530b63ee7fL115-L116),
[link](https://github.com/taichi-dev/taichi/pull/7851/files?diff=unified&w=0#diff-0290f4565dcad1ce884dc8e6377a55a36bdf13bc3914c2e9b9da039cd22dcdf1L765-L768))
* Replace call to `fetch_result_uint64` with direct access to result
buffer in `LlvmDevice` constructor
([link](https://github.com/taichi-dev/taichi/pull/7851/files?diff=unified&w=0#diff-84b10991213fe459611f39f3d35c0b6e33f7e9704c60fe6aeb0b4c4fff435ba5L11-R11))

---------

Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
quadpixels pushed a commit to quadpixels/taichi that referenced this issue May 13, 2023
)

Related: taichi-dev#6832

1. Removed the TI_XXX logging from Vulkan RHI and prepare for total
removal of spdlog from RHI
2. Started wrapping internal Vulkan RHI functions with `RhiReturn<>` to
return a error status instead of throwing exception
3. Start to work on the `implementation API` vs `public API` separation

Updating AOT demo as required:
taichi-dev/taichi-aot-demo#98

Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
quadpixels pushed a commit to quadpixels/taichi that referenced this issue May 13, 2023
…rResourceSet & RasterResources (taichi-dev#6954)

Issue: taichi-dev#6832

### Brief Summary

ResourceBinder is not split into two structures, one controls binding of
shared-accessible resources, the other is dedicated to control binding
of rasterizer states (vertex buffers, etc.)

This makes the mapping onto Vulkan DescriptorSets easier, and cleans up
the implementation of resource binding all over the place. In addition,
these binding states are now no longer attached to the program and can
be thus pre-filled / pre-allocated to achieve lower overhead.

Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
quadpixels pushed a commit to quadpixels/taichi that referenced this issue May 13, 2023
)

Issue: taichi-dev#6832

### Brief Summary

Compute-only CommandList APIs other than dispatch has been changed to
use `noexcept`, and the behavior specifications has been added, and
relevant checks are now in place for Vulkan and partially for DX11.

Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
quadpixels pushed a commit to quadpixels/taichi that referenced this issue May 13, 2023
Issue: taichi-dev#6832

Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
quadpixels pushed a commit to quadpixels/taichi that referenced this issue May 13, 2023
Issue: taichi-dev#6832

Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
quadpixels pushed a commit to quadpixels/taichi that referenced this issue May 13, 2023
…taichi-dev#7091)

Issue: taichi-dev#6832

### Brief Summary

The `create_pipeline` API has been updated to new standards.

In addition, we now added (optional) support for backend caches such as
`VkPipelineCache`. This makes running cached taichi programs even
faster.

Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
quadpixels pushed a commit to quadpixels/taichi that referenced this issue May 13, 2023
Issue: taichi-dev#6832

Allocate memory now returns a result code and runtime should validate
whether memory allocation succeeded or not

---------

Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
quadpixels pushed a commit to quadpixels/taichi that referenced this issue May 13, 2023
Issue: taichi-dev#6832

### Brief Summary

<!--
copilot:summary
-->
### <samp>🤖 Generated by Copilot at 8f04f4a</samp>

Remove the `fetch_result_uint64` method from the `Device` interface and
its subclasses, as it was redundant and inefficient. Use direct access
to the result buffer instead of copying or returning a single value.

### Walkthrough

<!--
copilot:walkthrough
-->
### <samp>🤖 Generated by Copilot at 8f04f4a</samp>

* Remove `fetch_result_uint64` method from `Device` interface and
subclasses, as it is redundant and inefficient
([link](https://github.com/taichi-dev/taichi/pull/7851/files?diff=unified&w=0#diff-e0843d34a17298595cd26f5b47b1933a0d18c5c795b0c91b73ec8541a2cdd3f9L142-L148),
[link](https://github.com/taichi-dev/taichi/pull/7851/files?diff=unified&w=0#diff-a7b85199b3c0a668f23bf0b723c126009e774437499d555de255883c1d1f4648L89-L90),
[link](https://github.com/taichi-dev/taichi/pull/7851/files?diff=unified&w=0#diff-e49f3cb85e1e9d5e83043e56f4d5d69c371bf8f40b4b653e31fc24eaea7e30c7L153-L157),
[link](https://github.com/taichi-dev/taichi/pull/7851/files?diff=unified&w=0#diff-36789bee03bd10430f6b60242918f8518e49eec53a9024c7ef6043de304712b4L101-L102),
[link](https://github.com/taichi-dev/taichi/pull/7851/files?diff=unified&w=0#diff-7919f5d7e33aafc72f27ed93febc58a0ac77c220ae718bf78ef134dad3790654L182-L188),
[link](https://github.com/taichi-dev/taichi/pull/7851/files?diff=unified&w=0#diff-01cbaa214a08d98caa33977f217853d54c85d51c03e2dab610909f530b63ee7fL115-L116),
[link](https://github.com/taichi-dev/taichi/pull/7851/files?diff=unified&w=0#diff-0290f4565dcad1ce884dc8e6377a55a36bdf13bc3914c2e9b9da039cd22dcdf1L765-L768))
* Replace call to `fetch_result_uint64` with direct access to result
buffer in `LlvmDevice` constructor
([link](https://github.com/taichi-dev/taichi/pull/7851/files?diff=unified&w=0#diff-84b10991213fe459611f39f3d35c0b6e33f7e9704c60fe6aeb0b4c4fff435ba5L11-R11))

---------

Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
doc Documentation related issues & PRs
Projects
Status: In Progress
Development

No branches or pull requests

1 participant