-
Notifications
You must be signed in to change notification settings - Fork 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
feat: implement F16 support in shaders #5701
base: trunk
Are you sure you want to change the base?
Conversation
I've marked this as ready for review, as the wgpu specific logic is implemented and would be great to start iterating on it. 2 main blockers are:
|
Dogfooding works! Fixed a few bugs in 30e12b5. Still waiting on upstream. |
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
One thing I'm concerned is that the polyfills we have in some of the backends might not support |
This comment was marked as resolved.
This comment was marked as resolved.
9c6a9b3
to
9f33e0e
Compare
This comment was marked as outdated.
This comment was marked as outdated.
You are indeed correct, thank you! |
808db24
to
aac66f9
Compare
This comment was marked as outdated.
This comment was marked as outdated.
aac66f9
to
64c94b8
Compare
Getting this over the line is on my list, will probably be next week. |
64c94b8
to
e68c86e
Compare
I've resolved conflicts with the latest |
e68c86e
to
16a41a2
Compare
8192315
to
d4909a2
Compare
I have split the hex parsing task into a different work item to follow this. See #7046. |
d4909a2
to
d57584d
Compare
Just realized that we didn't have the |
493e740
to
01bc1bd
Compare
Restored ownership of the commit, sorry about that :) |
man it's happening, let's go! ❤️ @cwfitzgerald @ErichDonGubler please lemme know if i can help somehow |
68ac734
to
efb441d
Compare
e6419c4
to
7b42714
Compare
pub(super) const fn check_width( | ||
&self, | ||
scalar: crate::Scalar, | ||
) -> Result<PushConstantCompatibility, WidthError> { | ||
let mut push_constant_compatibility = Ok(()); |
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.
This is the one change I wasn't sure about, but this solution does end up working fine.
naga/tests/example_wgsl.rs
Outdated
|
||
/// Runs through all example shaders and ensures they are valid wgsl. | ||
#[test] | ||
pub fn parse_example_wgsl() { | ||
let read_dir = match PathBuf::from(env!("CARGO_MANIFEST_DIR")) |
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.
This needs to be factored out.
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.
Split out as #7191
@@ -1,17 +1,17 @@ | |||
; SPIR-V | |||
; Version: 1.0 | |||
; Generator: rspirv | |||
; Bound: 125 | |||
; Bound: 123 |
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.
Why did this change?
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.
Oh I remember why - the old As
code ended up allocating an id
that it never used. The new code does not have this property, so there's some small renumberings in the other spvasm files.
7b42714
to
2343369
Compare
2343369
to
a84765e
Compare
Co-authored-by: FL33TW00D <[email protected]> Co-authored-by: ErichDonGubler <[email protected]>
a84765e
to
9c06280
Compare
Connections
enable …;
that reports nice errors #6424.enable
directive #5476.Checklist for merge-ability
hexf_parse f16 support: https://github.com/lifthrasiir/hexf. This one won't be fun. Did some work on it here: lifthrasiir/hexf@master...FL33TW00D:hexf:feature/f16 .Deferred to Implement Parsing of Hexadecimal F16 #7046f16
inside push constants.Additional context
F16 is available in >=SM6.2: https://github.com/microsoft/DirectXShaderCompiler/wiki/16-Bit-Scalar-Types