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

[WebGPU] Unify Emscripten and dawn-native paths in the runtime #7248

Open
jrprice opened this issue Dec 20, 2022 · 11 comments
Open

[WebGPU] Unify Emscripten and dawn-native paths in the runtime #7248

jrprice opened this issue Dec 20, 2022 · 11 comments

Comments

@jrprice
Copy link
Contributor

jrprice commented Dec 20, 2022

There are a few differences in the native APIs implemented by Emscripten and Dawn that mean Halide's WebGPU backend currently requires a build-time switch to select between the two. Once these differences are resolved, we should be able to remove this conditional logic and the build-time switch.

The differences are:

  1. Emscripten missing wgpuCreateInstance and wgpuReleaseInstance
  2. Emscripten missing wgpuAdapterGetLimits
  3. Waiting for async device init callbacks: emscripten_sleep() vs usleep() (e.g. wgpuInstanceProcessEvents())
  4. Waiting on other async callbacks: emscripten_sleep() vs wgpuDeviceTick() (e.g. wgpuInstanceProcessEvents())
@steven-johnson
Copy link
Contributor

Is there any plan as to when/if this will get resolved?

@steven-johnson
Copy link
Contributor

One comment in #6492 suggests that the lack of support for wgpuInstanceProcessEvents() is also tied to this issue, but I don't see it mentioned above

@jrprice
Copy link
Contributor Author

jrprice commented Feb 23, 2023

lack of support for wgpuInstanceProcessEvents() is also tied to this issue

Yeah, this is the expected name for the solution for the emscripten_sleep() vs wgpuDeviceTick() discrepancy. I've added it to the original comment to make that clearer.

Is there any plan as to when/if this will get resolved?

We're focusing on getting V1 of WebGPU shipped in Chrome really soon [1], and then we expect to have a renewed focus on making the native vs web story better. So yes, these things will get resolved, but I can't give an exact timeframe right now.

[1]: Intent to ship: WebGPU (Chromium)

@steven-johnson
Copy link
Contributor

Ah, gotcha, thanks. FYI, googling for wgpuInstanceProcessEvents() shows that it is in Emscripten's current version of webgpu.h: https://github.com/emscripten-core/emscripten/blob/main/system/include/webgpu/webgpu.h -- is it already implemented there?

@jrprice
Copy link
Contributor Author

jrprice commented Feb 23, 2023

is it already implemented there?

It's in the header (which I think is just a copy of Dawn's header), but the implementation is still absent:
https://github.com/emscripten-core/emscripten/blob/7de3bc93f0f7b65549215772255779ad8305ce78/src/library_webgpu.js#L2426

steven-johnson added a commit that referenced this issue Mar 14, 2023
Halide promises that you can crosscompile to *any* supported target from a 'stock' build of libHalide. Unfortunately, the initial landing of WebGPU support breaks that promise: we compile the webgpu runtime support (webgpu.cpp) with code that is predicated on `WITH_DAWN_NATIVE` (for Dawn vs Emscripten, respectively).

This means that if you build Halide with `WITH_DAWN_NATIVE` defined, you can *only* target Dawn with that build of Halide; similarly, if you build with `WITH_DAWN_NATIVE` not-defined, you can only target Emscripten. (Trying to use the 'wrong' version will produce link-time errors.)

For people who build everything from source, this isn't a big deal, but for people who just pull binary builds, this is a big problem.

This PR proposes a temporary workaround until the API discrepancies are resolved:
- Compile the existing webgpu.cpp runtime *both* ways
- in LLVM_Runtime_Linker.cpp, select the correct variant based on whether the Target is targeting wasm or not
- Profit!

This is a rather ugly hack, but it should hopefully be (relatively) temporary.
steven-johnson added a commit that referenced this issue Mar 16, 2023
* Split WebGPU runtime into two variants (#7248 workaround)

Halide promises that you can crosscompile to *any* supported target from a 'stock' build of libHalide. Unfortunately, the initial landing of WebGPU support breaks that promise: we compile the webgpu runtime support (webgpu.cpp) with code that is predicated on `WITH_DAWN_NATIVE` (for Dawn vs Emscripten, respectively).

This means that if you build Halide with `WITH_DAWN_NATIVE` defined, you can *only* target Dawn with that build of Halide; similarly, if you build with `WITH_DAWN_NATIVE` not-defined, you can only target Emscripten. (Trying to use the 'wrong' version will produce link-time errors.)

For people who build everything from source, this isn't a big deal, but for people who just pull binary builds, this is a big problem.

This PR proposes a temporary workaround until the API discrepancies are resolved:
- Compile the existing webgpu.cpp runtime *both* ways
- in LLVM_Runtime_Linker.cpp, select the correct variant based on whether the Target is targeting wasm or not
- Profit!

This is a rather ugly hack, but it should hopefully be (relatively) temporary.

* A few more fixes

* Update HalideGeneratorHelpers.cmake

* Update interpreter.cpp

* Update interpreter.cpp
@steven-johnson
Copy link
Contributor

steven-johnson commented Aug 24, 2023

As of Aug 2023, Emscripten/Dawn now support (and require) wgpuCreateInstance() and wgpuInstanceRelease() in non-native code, so I updated those in #7804. (wgpuInstanceProcessEvents() and wgpuAdapterGetLimits() are still unimplemented.)

@steven-johnson
Copy link
Contributor

As of now, wgpuInstanceProcessEvents() and wgpuAdapterGetLimits() are both present in webgpu.h -- are they still nonfunctional?

@jrprice
Copy link
Contributor Author

jrprice commented Nov 27, 2023

wgpuInstanceProcessEvents() and wgpuAdapterGetLimits() are still both unimplemented by Emscripten.

For future reference the Emscripten implementation is here, and it's usually easy to search for these functions since they are present but both just abort().

@steven-johnson
Copy link
Contributor

...sigh. Is there any path or plan for them to implement them?

@steven-johnson
Copy link
Contributor

Since WebGPU API issues have come up again, I see these are still unimplemented; is there any likely path to them getting there, or should we assume this is just a long-term API divergence?

@jrprice
Copy link
Contributor Author

jrprice commented Feb 8, 2024

I've asked the folks closer to this, and the response is still that we should eventually get to a world where you can target both native and Emscripten without any #ifdefs. I'm afraid we can't give a specific timeframe for when this will happen though. The latest header update PR did remove one such #ifdef however, so that's something!

ardier pushed a commit to ardier/Halide-mutation that referenced this issue Mar 3, 2024
…de#7419)

* Split WebGPU runtime into two variants (halide#7248 workaround)

Halide promises that you can crosscompile to *any* supported target from a 'stock' build of libHalide. Unfortunately, the initial landing of WebGPU support breaks that promise: we compile the webgpu runtime support (webgpu.cpp) with code that is predicated on `WITH_DAWN_NATIVE` (for Dawn vs Emscripten, respectively).

This means that if you build Halide with `WITH_DAWN_NATIVE` defined, you can *only* target Dawn with that build of Halide; similarly, if you build with `WITH_DAWN_NATIVE` not-defined, you can only target Emscripten. (Trying to use the 'wrong' version will produce link-time errors.)

For people who build everything from source, this isn't a big deal, but for people who just pull binary builds, this is a big problem.

This PR proposes a temporary workaround until the API discrepancies are resolved:
- Compile the existing webgpu.cpp runtime *both* ways
- in LLVM_Runtime_Linker.cpp, select the correct variant based on whether the Target is targeting wasm or not
- Profit!

This is a rather ugly hack, but it should hopefully be (relatively) temporary.

* A few more fixes

* Update HalideGeneratorHelpers.cmake

* Update interpreter.cpp

* Update interpreter.cpp
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

No branches or pull requests

2 participants