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

Naga's typechecker ignores scalar kind when casting matrices #6441

Closed
cogumbreiro opened this issue Oct 22, 2024 · 3 comments
Closed

Naga's typechecker ignores scalar kind when casting matrices #6441

cogumbreiro opened this issue Oct 22, 2024 · 3 comments
Labels
naga Shader Translator

Comments

@cogumbreiro
Copy link

cogumbreiro commented Oct 22, 2024

Description

I think that there's a bug in the naga/src/proc/typifier.rs. Notably, when casting with As the new scalar kind, variable kind, is not propagated to the output scalar, only the width.

                Ti::Matrix {
                    columns,
                    rows,
                    mut scalar,
                } => {
                    if let Some(width) = convert {
                        scalar.width = width;
                    }
                    TypeResolution::Value(Ti::Matrix {
                        columns,
                        rows,
                        scalar,
                    })
                }

Note a correct behavior on vectors where the kind is propagated to the generated vector:

                Ti::Vector {
                    size,
                    scalar: crate::Scalar { kind: _, width },
                } => TypeResolution::Value(Ti::Vector {
                    size,
                    scalar: crate::Scalar {
                        kind,
                        width: convert.unwrap_or(width),
                    },
                }), 

Source:

TypeResolution::Value(Ti::Matrix {
columns,
rows,
scalar,

@cogumbreiro
Copy link
Author

I've added an example that crashes in current naga.

@cogumbreiro
Copy link
Author

cogumbreiro commented Oct 24, 2024

I just realized that according to the spec, a matrix can have f32 or f16. However, since naga doesn't support f16, then perhaps this is not really a bug as of now.

However, here's a minimal example for posterity which I'm guessing would break once support for f16 is implemented:

@compute @workgroup_size(256, 1, 1) fn computeSomething() {
  let m1: mat2x2<f32> = mat2x2<f32>(
      vec2<f32>(1, 2),
      vec2<f32>(3, 4)
  );

  let m2: mat2x2<f16> = mat2x2<f16>(m1);
}

@teoxoy teoxoy added the naga Shader Translator label Oct 24, 2024
@jimblandy
Copy link
Member

Since matrices are enforced to be f32 by the validator, this isn't really a bug we can hit at the moment. When we add f16 matrix support, this will become relevant.

@github-project-automation github-project-automation bot moved this from Todo to Done in WebGPU for Firefox Nov 6, 2024
@teoxoy teoxoy closed this as not planned Won't fix, can't repro, duplicate, stale Nov 6, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
naga Shader Translator
Projects
Status: Done
Development

No branches or pull requests

3 participants