Skip to content

Roll Emdawnwebgpu and repoint USE_WEBGPU tests at Emdawnwebgpu #25397

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.

Sign up for GitHub

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

Open
wants to merge 6 commits into
base: main
Choose a base branch
from
+165 −287
Open

Roll Emdawnwebgpu and repoint USE_WEBGPU tests at Emdawnwebgpu #25397

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
595792c
Roll Emdawnwebgpu and repoint USE_WEBGPU tests at Emdawnwebgpu
kainino0x Sep 26, 2025
d12517b
stronger test that keepalives work correctly
kainino0x Sep 26, 2025
ef5a6da
Roll Emdawnwebgpu
kainino0x Sep 27, 2025
4f93d5f
Skip with FROZEN_CACHE
kainino0x Sep 27, 2025
3d62df6
Remove unneeded NO_DEFAULT_TO_CXX
kainino0x Sep 29, 2025
e817b60
Merge branch 'main' into roll-and-repoint-tests
kainino0x Sep 30, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/jsifier.mjs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -284,7 +284,7 @@ function handleI64Signatures(symbol, snippet, sig, i53abi) {
const newArgs = [];
let argConversions = '';
if (sig.length > argNames.length + 1) {
error(`handleI64Signatures: signature too long for ${symbol}`);
error(`handleI64Signatures: signature '${sig}' too long for ${symbol}(${argNames.join(', ')})`);
return snippet;
}
for (let i = 0; i < argNames.length; i++) {
Expand Down
16 changes: 4 additions & 12 deletions test/test_browser.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4463,24 +4463,16 @@ def test_webgl_simple_extensions(self, webgl_version, simple_enable_extensions):
'closure': (['-sASSERTIONS', '--closure=1'],),
'closure_advanced': (['-sASSERTIONS', '--closure=1', '-O3'],),
'main_module': (['-sMAIN_MODULE=1'],),
'pthreads': (['-pthread', '-sOFFSCREENCANVAS_SUPPORT'],),
})
@requires_webgpu
def test_webgpu_basic_rendering(self, args):
self.btest_exit('webgpu_basic_rendering.cpp', cflags=['-Wno-error=deprecated', '-sUSE_WEBGPU'] + args)
self.btest_exit('webgpu_basic_rendering.cpp', cflags=['--use-port=emdawnwebgpu', '-sEXIT_RUNTIME'] + args)

@requires_webgpu
def test_webgpu_required_limits(self):
self.btest_exit('webgpu_required_limits.c', cflags=['-Wno-error=deprecated', '-sUSE_WEBGPU', '-sASYNCIFY'])

@requires_webgpu
def test_webgpu_basic_rendering_pthreads(self):
self.btest_exit('webgpu_basic_rendering.cpp', cflags=['-Wno-error=deprecated', '-sUSE_WEBGPU', '-pthread', '-sOFFSCREENCANVAS_SUPPORT'])

def test_webgpu_get_device(self):
self.btest_exit('webgpu_get_device.cpp', cflags=['-Wno-error=deprecated', '-sUSE_WEBGPU', '-sASSERTIONS', '--closure=1'])

def test_webgpu_get_device_pthreads(self):
self.btest_exit('webgpu_get_device.cpp', cflags=['-Wno-error=deprecated', '-sUSE_WEBGPU', '-pthread'])
self.set_setting('NO_DEFAULT_TO_CXX', 0) # emdawnwebgpu uses C++ internally
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What to do about this long term?

Is this some fundamental difference between the old and new implementation? The only one manged to avoid depending on libc++ library functions but the new one did not? Are folks who use this library simply bound to link with C++?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great question. I'm pretty sure your guess is correct - the old one only used #include <array> which probably didn't require any linking, but now we use a whole bunch of stuff, in particular <atomic> and <mutex>, at least one of which I believe requires linking.

Technically we could remove those dependencies in single-threaded builds, but it's much easier for us to just rely on the compiler to handle those optimizations. (Though, mutexes might actually not be required until we have native multithreading in the browser's WebGPU but I'm not sure off hand. Atomics are used for refcounts which are needed even today.)

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do <atomic> and <mutex> end up requiring library symbols at link time? If not maybe it could be a goal to avoid link-time libc++ symbols? Might be worth a try to see how hard it would be.

Copy link
Collaborator Author

@kainino0x kainino0x Sep 29, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh right, I suppose I can check. I totally forgot what the original error message was that led me to add this.

wasm-ld: error: /Users/kainino/src/emscripten/cache/sysroot/lib/wasm32-emscripten/libemdawnwebgpu-ac372172-O0.a(webgpu.cpp.o): undefined symbol: operator new(unsigned long)
wasm-ld: error: /Users/kainino/src/emscripten/cache/sysroot/lib/wasm32-emscripten/libemdawnwebgpu-ac372172-O0.a(webgpu.cpp.o): undefined symbol: operator delete(void*, unsigned long)
... many copies of those lines

Those we certainly can't easily get rid of...

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think maybe you can? Maybe either by defining your own local versions of new and delete, or switching the placement new/delete?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's probably possible, I (mostly) figured out how to overload the new/delete operators - but I read the output properly and realized it said I should pass -error-limit=0, so figured out how to do that (-Wl,--error-limit=0) and here's the full (deduplicated) list:

undefined symbol: __cxa_pure_virtual
undefined symbol: operator delete(void*, unsigned long, std::align_val_t)
undefined symbol: operator delete(void*, unsigned long)
undefined symbol: operator new(unsigned long, std::align_val_t)
undefined symbol: operator new(unsigned long)
undefined symbol: std::__2::__libcpp_verbose_abort(char const*, ...)
undefined symbol: std::__2::__next_prime(unsigned long)
undefined symbol: std::__2::basic_string<char, std::__2::char_traits<char>, std::__2::allocator<char>>::__assign_external(char const*)
undefined symbol: std::__2::basic_string<char, std::__2::char_traits<char>, std::__2::allocator<char>>::~basic_string()
undefined symbol: std::__2::mutex::~mutex()
undefined symbol: std::__2::mutex::lock()
undefined symbol: std::__2::mutex::unlock()
undefined symbol: vtable for __cxxabiv1::__class_type_info
undefined symbol: vtable for __cxxabiv1::__si_class_type_info
undefined symbol: vtable for __cxxabiv1::__vmi_class_type_info

(TBH not sure where some of these things came from (like __class_type_info), there must be something in the STL.) Regardless I don't think it is really practical to avoid using libc++.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see, yes if you are using STL types then you will need libc++ linked in.

The question is, is it intended to be able to use this port from C programs? I suppose the answer is yes, in which case we need to find a way to add -lc++ when using the port. I believe this is the first port with this requirement.

I imagine the right place to perform this injection would be process_dependencies but I'm not sure we have a mechanism to inject link flags like this.

Oh way.. I think you can do settings.LINK_AS_CXX = true in the function! Can you try that?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh great idea, that works! It overrides NO_DEFAULT_TO_CXX for the rest of the program... but certainly easier to use. Will send a Dawn CL.

self.btest_exit('webgpu_required_limits.c', cflags=['--use-port=emdawnwebgpu', '-sEXIT_RUNTIME'])

# Tests the feature that shell html page can preallocate the typed array and place it
# to Module.buffer before loading the script page.
Expand Down
22 changes: 9 additions & 13 deletions test/test_other.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9585,13 +9585,15 @@ def test_closure_full_js_library(self, args):

@also_with_wasm64
def test_closure_webgpu(self):
# This test can be removed if USE_WEBGPU is later included in INCLUDE_FULL_LIBRARY.
if config.FROZEN_CACHE:
# TODO(crbug.com/446944885): Make Emdawnwebgpu work with FROZEN_CACHE if possible.
self.skipTest("test doesn't work with frozen cache")
self.set_setting('NO_DEFAULT_TO_CXX', 0) # emdawnwebgpu uses C++ internally
self.build('hello_world.c', cflags=[
'--closure=1',
'-Werror=closure',
'-Wno-error=deprecated',
'-sINCLUDE_FULL_LIBRARY',
'-sUSE_WEBGPU',
'--use-port=emdawnwebgpu',
])

# Tests --closure-args command line flag
Expand Down Expand Up @@ -12230,15 +12232,6 @@ def test_standalone_syscalls(self):
for engine in config.WASM_ENGINES:
self.assertContained(expected, self.run_js('test.wasm', engine))

@parameterized({
'': ([],),
'assertions': (['-sASSERTIONS'],),
'closure': (['-sASSERTIONS', '--closure=1'],),
'dylink': (['-sMAIN_MODULE'],),
})
def test_webgpu_compiletest(self, args):
self.run_process([EMXX, test_file('webgpu_jsvalstore.cpp'), '-Wno-error=deprecated', '-sUSE_WEBGPU', '-sASYNCIFY'] + args)

@flaky('https://github.com/emscripten-core/emscripten/issues/25343')
@also_with_wasm64
@parameterized({
Expand All @@ -12247,7 +12240,10 @@ def test_webgpu_compiletest(self, args):
'closure_assertions': (['--closure=1', '-Werror=closure', '-sASSERTIONS'],),
})
def test_emdawnwebgpu_link_test(self, args):
self.run_process([EMXX, test_file('test_emdawnwebgpu_link_test.cpp'), '--use-port=emdawnwebgpu', '-sASYNCIFY'] + args)
if config.FROZEN_CACHE:
# TODO(crbug.com/446944885): Make Emdawnwebgpu work with FROZEN_CACHE if possible.
self.skipTest("test doesn't work with frozen cache")
self.emcc(test_file('test_emdawnwebgpu_link_test.cpp'), ['--use-port=emdawnwebgpu', '-sASYNCIFY'] + args)

def test_signature_mismatch(self):
create_file('a.c', 'void foo(); int main() { foo(); return 0; }')
Expand Down
Loading