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

Runtime: Add float16 support to bigarray #1803

Merged
merged 7 commits into from
Jan 23, 2025
Merged

Runtime: Add float16 support to bigarray #1803

merged 7 commits into from
Jan 23, 2025

Conversation

hhugo
Copy link
Member

@hhugo hhugo commented Jan 17, 2025

float16array is not widely supported.

@hhugo
Copy link
Member Author

hhugo commented Jan 17, 2025

@vouillon, I think the wasm runtime needs to be updated somehow. Can you take a look ?

@hhugo hhugo force-pushed the float16 branch 3 times, most recently from fc6d04c to 2c27325 Compare January 20, 2025 10:07
@hhugo hhugo marked this pull request as ready for review January 20, 2025 10:07
@hhugo hhugo added the bug label Jan 20, 2025
@hhugo hhugo added this to the 6.0 milestone Jan 20, 2025
@hhugo
Copy link
Member Author

hhugo commented Jan 20, 2025

@vouillon, I think there is a bug in the wasm runtime of bigarrays plus we need to add support for float16.
I'd like to include this in the 6.0 release. Can you take a look ?

@hhugo
Copy link
Member Author

hhugo commented Jan 21, 2025

Thanks for the wasm support.
The js runtime uses Uint16Array which has the same memory layout as float16,
The wasm runtime uses Float32Array which has a larger memory footprint but saves us some conversion.
@vouillon, do you think we can leave it like this ? Should we unify the two ?

@vouillon
Copy link
Member

@hhugo Maybe I should switch to using Uint16array typed arrays, actually. Eventually, we should use Float16Array, but it's still under a flag in Chrome, and cannot be used with WebGL in Safari:
https://github.com/WebKit/WebKit/blob/9fca11bf1d0cc85c923d86c2a8e71045c30dfc98/Source/WebCore/html/canvas/WebGLRenderingContextBase.cpp#L3767-L3769

@hhugo
Copy link
Member Author

hhugo commented Jan 22, 2025

@hhugo Maybe I should switch to using Uint16array typed arrays, actually. Eventually, we should use Float16Array, but it's still under a flag in Chrome, and cannot be used with WebGL in Safari: https://github.com/WebKit/WebKit/blob/9fca11bf1d0cc85c923d86c2a8e71045c30dfc98/Source/WebCore/html/canvas/WebGLRenderingContextBase.cpp#L3767-L3769

Should I wait for this change ?

@hhugo
Copy link
Member Author

hhugo commented Jan 22, 2025

(I've a commit that uses float16array in js if available but maybe we should sit on it given the safari situation, 5099f19)

@vouillon
Copy link
Member

Should I wait for this change ?

I'll try to do that tomorrow

hhugo and others added 4 commits January 23, 2025 13:16
@hhugo hhugo merged commit 5f1ca29 into master Jan 23, 2025
30 checks passed
@hhugo hhugo deleted the float16 branch January 23, 2025 13:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants