-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
adds mappings for f16 variants of halide float math #8029
adds mappings for f16 variants of halide float math #8029
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we need a test for this?
Im happy to write one... though I didn't see any coverage of the existing f32 codegen ( correctness/float16_t.cpp has coverage of those intrinsics, but only under cpu scheduling, and a simple test of metal roundtripping ) |
Doesn't need to be done as part of this PR, but IMHO we clearly need to extend this test to be executed under GPU since we expect those features to work. If you don't have the bandwidth to do so, let's get someone else to do it.. |
Update: I'm gonna land this and take it upon myself to add the right tests |
Tweak correctness_float16_t so that it uses one of the transcendal functions (sqrt) that were missing in Metal.
* adds mappings for f16 variants of halide float math * fix clang format errors * trigger buildbots --------- Co-authored-by: Steven Johnson <srj@google.com>
Tweak correctness_float16_t so that it uses one of the transcendal functions (sqrt) that were missing in Metal.
Currently you can define float16 variables in a function with a metal schedule, but if codegen emits any float16 math intrincs, the metal compilation will fail because only f32 variants were defined. This adds f16 variants.