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

Remove WebGPU native platform support. #10646

Merged
merged 3 commits into from
Oct 4, 2022

Conversation

ScottTodd
Copy link
Member

This removes the Dawn and wgpu-native implementations of WebGPU. I kept the iree/hal/drivers/webgpu/platform/webgpu.h file and platform/ file structure so that we can still build most of the WebGPU HAL using webgpu-headers. I also emptied out the hal/drivers/webgpu/registration/driver_module_native.c implementation to just return IREE_STATUS_UNIMPLEMENTED.

Dawn and wgpu-native are both very large dependencies that we would only use for development and testing (we have direct support for Vulkan, CUDA, etc., so we wouldn't use them as compatibility layers on native platforms). We'll focus our efforts on the browser implementation of WebGPU (using Emscripten, at least to start).

@ScottTodd ScottTodd added hal/webgpu Runtime WebGPU HAL backend platform/web 🌐 Web-specific build, execution, benchmarking, and deployment labels Oct 4, 2022
@ScottTodd
Copy link
Member Author

Sweet, the Bazel build works now too :)

@ScottTodd ScottTodd merged commit 00fda76 into iree-org:webgpu Oct 4, 2022
@ScottTodd ScottTodd deleted the webgpu-drop-native branch October 4, 2022 22:49
@GMNGeoffrey
Copy link
Contributor

I suspect this is indeed the right call, but anytime I see "delete this thingy we previously built" it worries me a bit :-) When we originally added this, was it with the intention that it would maybe be deleted later, but it was still worth it for what we'd learn? Some of this was in experimental, so I guess it was just an experiment? What about the stuff in the main directories? What changed between when we added it and when we're now deleting it? How much effort did we put into creating this and did we get enough information from doing so that the investment was "worth it"?

@ScottTodd
Copy link
Member Author

Note that this is still on a branch (was benvanik-webgpu-skeleton, now just webgpu), not main. Part of the intent here is to clean up the branch prior to merging the work into main.

When we originally added this, was it with the intention that it would maybe be deleted later, but it was still worth it for what we'd learn? Some of this was in experimental, so I guess it was just an experiment?

There are a few dimensions to this. WebGPU as an API is still under development, and we started work on this branch (the WebGPU HAL runtime code) over a year ago, when it was even less complete. Chrome is implementing their support for the API using Dawn. Firefox is building/using wgpu and wgpu-native. We figured that bootstrapping with one of the native implementations would be easier than jumping straight to a web implementation (and using Emscripten's webgpu bindings, which have also been under development).

It could also be useful to compare performance between

  • a native WebGPU implementation (backed by Vulkan)
  • a browser WebGPU implementation (backed by Vulkan, with browser overheads)
  • a native Vulkan implementation

However, Dawn depends on absl (which affects compiler options in weird ways), and wgpu-native uses Rust. Both have their own additional dependencies, and supporting either or both in addition to what we already support would be tricky.

What changed between when we added it and when we're now deleting it?

Time passed - our web platform support has gotten further along, the WebGPU API got closer to being finalized, and (early adopter) implementations of WebGPU in browsers are more easy to use now.

How much effort did we put into creating this and did we get enough information from doing so that the investment was "worth it"?

A few weeks - a month of Ben's focused development time maybe? It's not lost work - most of the code is common to the browser implementation. The early work also uncovered a bunch of pain points and other feedback for the API: #6499 (and the linked feedback on https://github.com/gpuweb/gpuweb).

@benvanik
Copy link
Collaborator

benvanik commented Oct 5, 2022

👍 I would not have been able to build out the implementation without doing it like this and used it to guide the massive reworkings of things resulting in iree_loop_t, the async HAL, and stream dialect. Thankfully I built things in such a way that it could be cleanly deleted/added again: as a habit most of the code I write is written to be easily deleted - I hate needing to keep crusty marginally-useful code alive more than I hate deleting my work :)

I still think long term there is value in a native webgpu path for testing but only when there's a dedicated group of maintainers willing to keep it live and the dependencies up to date. For now every bit of effort we spend on that path now that we are getting the browser set up would be a distraction.

@GMNGeoffrey
Copy link
Contributor

Yeah I'm less asking why we're deleting and more why we built it in the first place (as an actual question, not an an accusation 🙂) and whether when we built it we did it knowing that there was a good chance of deleting it. It sounds like there was. I also missed that this wasn't on the main branch, which indeed suggests impermanence. Comment just intended as a mini post-mortem. (One focused-Ben-month actually sounds pretty expensive to me. That's like 3 Xbox emulators right there 😝)

ScottTodd added a commit that referenced this pull request Nov 2, 2022
This removes the [Dawn](https://dawn.googlesource.com/dawn) and
[wgpu-native](https://github.com/gfx-rs/wgpu-native) implementations of
WebGPU. I kept the `iree/hal/drivers/webgpu/platform/webgpu.h` file and
`platform/` file structure so that we can still _build_ most of the
WebGPU HAL using
[webgpu-headers](https://github.com/webgpu-native/webgpu-headers). I
also emptied out the
`hal/drivers/webgpu/registration/driver_module_native.c` implementation
to just return `IREE_STATUS_UNIMPLEMENTED`.

Dawn and wgpu-native are both very large dependencies that we would only
use for development and testing (we have direct support for Vulkan,
CUDA, etc., so we wouldn't use them as compatibility layers on native
platforms). We'll focus our efforts on the browser implementation of
WebGPU (using Emscripten, at least to start).
ScottTodd added a commit that referenced this pull request Dec 2, 2022
This removes the [Dawn](https://dawn.googlesource.com/dawn) and
[wgpu-native](https://github.com/gfx-rs/wgpu-native) implementations of
WebGPU. I kept the `iree/hal/drivers/webgpu/platform/webgpu.h` file and
`platform/` file structure so that we can still _build_ most of the
WebGPU HAL using
[webgpu-headers](https://github.com/webgpu-native/webgpu-headers). I
also emptied out the
`hal/drivers/webgpu/registration/driver_module_native.c` implementation
to just return `IREE_STATUS_UNIMPLEMENTED`.

Dawn and wgpu-native are both very large dependencies that we would only
use for development and testing (we have direct support for Vulkan,
CUDA, etc., so we wouldn't use them as compatibility layers on native
platforms). We'll focus our efforts on the browser implementation of
WebGPU (using Emscripten, at least to start).
ScottTodd added a commit to ScottTodd/iree that referenced this pull request Apr 25, 2023
This removes the [Dawn](https://dawn.googlesource.com/dawn) and
[wgpu-native](https://github.com/gfx-rs/wgpu-native) implementations of
WebGPU. I kept the `iree/hal/drivers/webgpu/platform/webgpu.h` file and
`platform/` file structure so that we can still _build_ most of the
WebGPU HAL using
[webgpu-headers](https://github.com/webgpu-native/webgpu-headers). I
also emptied out the
`hal/drivers/webgpu/registration/driver_module_native.c` implementation
to just return `IREE_STATUS_UNIMPLEMENTED`.

Dawn and wgpu-native are both very large dependencies that we would only
use for development and testing (we have direct support for Vulkan,
CUDA, etc., so we wouldn't use them as compatibility layers on native
platforms). We'll focus our efforts on the browser implementation of
WebGPU (using Emscripten, at least to start).
ScottTodd added a commit to ScottTodd/iree that referenced this pull request May 18, 2023
This removes the [Dawn](https://dawn.googlesource.com/dawn) and
[wgpu-native](https://github.com/gfx-rs/wgpu-native) implementations of
WebGPU. I kept the `iree/hal/drivers/webgpu/platform/webgpu.h` file and
`platform/` file structure so that we can still _build_ most of the
WebGPU HAL using
[webgpu-headers](https://github.com/webgpu-native/webgpu-headers). I
also emptied out the
`hal/drivers/webgpu/registration/driver_module_native.c` implementation
to just return `IREE_STATUS_UNIMPLEMENTED`.

Dawn and wgpu-native are both very large dependencies that we would only
use for development and testing (we have direct support for Vulkan,
CUDA, etc., so we wouldn't use them as compatibility layers on native
platforms). We'll focus our efforts on the browser implementation of
WebGPU (using Emscripten, at least to start).
ScottTodd added a commit to ScottTodd/iree that referenced this pull request May 19, 2023
This removes the [Dawn](https://dawn.googlesource.com/dawn) and
[wgpu-native](https://github.com/gfx-rs/wgpu-native) implementations of
WebGPU. I kept the `iree/hal/drivers/webgpu/platform/webgpu.h` file and
`platform/` file structure so that we can still _build_ most of the
WebGPU HAL using
[webgpu-headers](https://github.com/webgpu-native/webgpu-headers). I
also emptied out the
`hal/drivers/webgpu/registration/driver_module_native.c` implementation
to just return `IREE_STATUS_UNIMPLEMENTED`.

Dawn and wgpu-native are both very large dependencies that we would only
use for development and testing (we have direct support for Vulkan,
CUDA, etc., so we wouldn't use them as compatibility layers on native
platforms). We'll focus our efforts on the browser implementation of
WebGPU (using Emscripten, at least to start).
ScottTodd added a commit to ScottTodd/iree that referenced this pull request May 23, 2023
This removes the [Dawn](https://dawn.googlesource.com/dawn) and
[wgpu-native](https://github.com/gfx-rs/wgpu-native) implementations of
WebGPU. I kept the `iree/hal/drivers/webgpu/platform/webgpu.h` file and
`platform/` file structure so that we can still _build_ most of the
WebGPU HAL using
[webgpu-headers](https://github.com/webgpu-native/webgpu-headers). I
also emptied out the
`hal/drivers/webgpu/registration/driver_module_native.c` implementation
to just return `IREE_STATUS_UNIMPLEMENTED`.

Dawn and wgpu-native are both very large dependencies that we would only
use for development and testing (we have direct support for Vulkan,
CUDA, etc., so we wouldn't use them as compatibility layers on native
platforms). We'll focus our efforts on the browser implementation of
WebGPU (using Emscripten, at least to start).
ScottTodd added a commit that referenced this pull request May 25, 2023
This removes the [Dawn](https://dawn.googlesource.com/dawn) and
[wgpu-native](https://github.com/gfx-rs/wgpu-native) implementations of
WebGPU. I kept the `iree/hal/drivers/webgpu/platform/webgpu.h` file and
`platform/` file structure so that we can still _build_ most of the
WebGPU HAL using
[webgpu-headers](https://github.com/webgpu-native/webgpu-headers). I
also emptied out the
`hal/drivers/webgpu/registration/driver_module_native.c` implementation
to just return `IREE_STATUS_UNIMPLEMENTED`.

Dawn and wgpu-native are both very large dependencies that we would only
use for development and testing (we have direct support for Vulkan,
CUDA, etc., so we wouldn't use them as compatibility layers on native
platforms). We'll focus our efforts on the browser implementation of
WebGPU (using Emscripten, at least to start).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
hal/webgpu Runtime WebGPU HAL backend platform/web 🌐 Web-specific build, execution, benchmarking, and deployment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants