Skip to content

Remove @requires_sync_compilation test annotation. NFC #20101

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

Merged
merged 1 commit into from
Aug 23, 2023

Conversation

sbc100
Copy link
Collaborator

@sbc100 sbc100 commented Aug 22, 2023

None of these tests require sync compilation anymore as far as I can tell, and even if they did, chrome now supports sync compilation up of wasm files up to 8Mb:
chromium/chromium@d1a1a8f

Also, rename test_binaryen_async to test_async_compile.

Fixes: #14404

@sbc100 sbc100 requested a review from kripken August 23, 2023 01:41
@sbc100 sbc100 force-pushed the remove_requires_sync_compilation branch from ac6af27 to 8c8dcc2 Compare August 23, 2023 01:43
None of these tests require sync compilation anymore as far as I can
tell, and even if they did, chrome now supports sync compilation up of
wasm files up to 8Mb:
chromium/chromium@d1a1a8f

Also, rename `test_binaryen_async` to `test_async_compile`.

Fixes: #14404
@sbc100 sbc100 force-pushed the remove_requires_sync_compilation branch from 8c8dcc2 to f41a41f Compare August 23, 2023 16:19
@sbc100 sbc100 requested a review from tlively August 23, 2023 17:39
@@ -2073,7 +2072,7 @@ def test_cubegeom_pre_regal(self):
self.btest('third_party/cubegeom/cubegeom_pre.c', reference='third_party/cubegeom/cubegeom_pre.png', args=['-sUSE_REGAL', '-DUSE_REGAL', '-lGL', '-lSDL'])

@requires_graphics_hardware
@requires_sync_compilation
@no_swiftshader
Copy link
Member

Choose a reason for hiding this comment

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

What is this for?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

If you look at all the other cubegeom_pre.c around this one.. they are all marked as @no_swiftshader. This particular one was never been run under swift shader before due to @requires_sync_compilation .. not that it is running, it fails, due to the swiftshader issue (I assume) so I added the same tag as the surrounding test.

Copy link
Member

@kripken kripken left a comment

Choose a reason for hiding this comment

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

lgtm % question

@sbc100 sbc100 merged commit 76652b9 into main Aug 23, 2023
@sbc100 sbc100 deleted the remove_requires_sync_compilation branch August 23, 2023 19:26
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.

Remove use of @requires_sync_compilation
2 participants