(Naga) Implement OpSpecConstantOp for the SPIR-V frontend

[hasenbanck]

Connections

https://github.com/gfx-rs/wgpu/issues/4324

Description

Implement OpSpecConstantOp for the SPIR-V frontend.

This implementation solves all my problems that I had with SLANGC output. But I couldn't implement all operations. The following are missing:

• Op::VectorShuffle, Op::CompositeExtract, Op::CompositeInsert => All work on non-scalars, which I can't get to work.
• Op::QuantizeToF16 => We don't support QuantizeToF16 in the SPIR-V frontend yet. I think the constant evaluator might also have problems with it.

We also properly convert implicit type conversions to explicit type conversions, convert signed shifts to unsigned shifts and also optimize common type conversion patterns produced by the GLSL compiler or SLANG.

Testing

I added two snapshot tests. One based on GLSL code and one in pure SPVASM, that tests more complex chained OpSpecConstantOps.

Squash or Rebase?

Rebase

Checklist

• [x] Run cargo fmt.
• [x] Run taplo format.
• [x] Run cargo clippy --tests. If applicable, add:
• [x] Run cargo xtask test to run tests.
• [x] If this contains user-facing changes, add a CHANGELOG.md entry.

[hasenbanck]

I added a second commit to the PR that removes a workaround and instead uses the same approach as the GLSL frontend, by properly using the ConstantEvaluatorto evaluate the constant expressions. This is a more invasive change, so I decide it to add as a second commit, but could be spun into it's own PR if the reviewer think that's the better option.

This would enable us in a future PR to extend the ConstantEvaluator to have a mode for SPIR-V, that takes care for the corner cases that compiler could create (like the corner cases by the GLSL compiler described above).

[hasenbanck]

Since no reviewer has assigned the PR to them, I took the liberty and added a third commit that polishes the implementation:

Implicit type conversions are now made explicit, so that we can now properly convert OpSpecConstantOp created by GLSL, that contain implicit type conversion properly into stricter forms, so that WGSL can validate and evaluate the overrides correctly.

We now also properly use the best available name for the override and also detect and optimize type conversion patterns that compiler like GLSL love to create. This improves debuggability of the produced shaders greatly.

I will update the PR text accordingly.

[hasenbanck]

Hi! It has been 4 weeks since I pushed this PR. Is there any way I could help get this PR faster through the pipeline?

[inner-daemons]

Just wanted to say, thanks for putting up this PR and thanks for your patience! This seems very valuable to have. It has been reassigned so hopefully it will get in soon.

[hasenbanck]

Thanks for the review! I removed the try_optimize_x() and answered your comments in the code.

But I don't properly understand this point:

We now have 2 places where we have to handle the same operation type and make sure that their semantics get translated to our IR's semantics in the same way. Please try to refactor the code to make both code-paths use the same logic.

What do you mean by this? Currently we have two code paths next_block() and parse_spec_constant_op() that convert the sematics in very different contexts. Merging their behavior would complicate the code in my eyes and I don't think that I would be the right person to do so. If you want to have them merged, then I kindly ask you to take over that task, since you have a clearer vision on how you want to have the code structured.

[inner-daemons]

@hasenbanck Can you resolve the conflicts and merge in the latest commits from trunk? The PR is quite old at this point and you will likely have to make a few tweaks.

[hasenbanck]

@hasenbanck Can you resolve the conflicts and merge in the latest commits from trunk? The PR is quite old at this point and you will likely have to make a few tweaks.

Done.

[teoxoy]

Thanks for the review! I removed the try_optimize_x() and answered your comments in the code.

But I don't properly understand this point:

We now have 2 places where we have to handle the same operation type and make sure that their semantics get translated to our IR's semantics in the same way. Please try to refactor the code to make both code-paths use the same logic.

What do you mean by this? Currently we have two code paths next_block() and parse_spec_constant_op() that convert the sematics in very different contexts. Merging their behavior would complicate the code in my eyes and I don't think that I would be the right person to do so. If you want to have them merged, then I kindly ask you to take over that task, since you have a clearer vision on how you want to have the code structured.

I feel like the context (const vs non const) matters very little in this case, it's mostly a matter of what expression arena is used to insert the expressions into. The semantics of the operations must be the same in both contexts per the SPIR-V spec. From what I can see most of the code in this PR is replicating the semantics of the existing operations rather than managing differences in the 2 contexts.

One of the most tedious things we recently had to deal with and are still working on was deduplicating code that diverged differently over time, which this code is prone to from the very start.