[DO NOT MERGE] Switch to Rust version of spirv-tools#521
Draft
LegNeato wants to merge 100 commits intoRust-GPU:mainfrom
Draft
[DO NOT MERGE] Switch to Rust version of spirv-tools#521LegNeato wants to merge 100 commits intoRust-GPU:mainfrom
LegNeato wants to merge 100 commits intoRust-GPU:mainfrom
Conversation
LegNeato
force-pushed
the
spirvopt
branch
3 times, most recently
from
f0b21ad to
4077424
Compare
Collaborator
Author
|
@nazar-pc feel free to try out this branch now that CI is passing, likely large issues lurking still though. |
Contributor
|
Looks like this PR is based on the old |
The published rspirv 0.12.0+sdk-1.3.268.0 doesn't include newer SPIR-V opcodes like UntypedPointersKHR, TypeUntypedPointerKHR, UntypedVariableKHR, and ConditionalEntryPointINTEL that are required by the spirv-tools crate. This patches rspirv and spirv to use the gfx-rs/rspirv sdk-update branch which includes these opcodes (version 0.12.0+sdk-1.4.335.0).
Switch from crates.io spirv-tools 0.13.0 to the Rust-GPU/SPIRV-Tools repository via git dependency. This provides access to the new pure-Rust SPIR-V validator and egglog-based optimizer. When using as a git dependency, set SPIRV_HEADERS_PATH to point to a local SPIRV-Headers checkout since the submodule isn't available.
Adapt to changes in rspirv 0.12.0+sdk-1.4.335.0: - Replace `rspirv::grammar::reflect::is_type(op)` with `op.is_type()` - Replace `rspirv::grammar::reflect::is_constant(op)` with `op.is_constant()` - Update `type_float_id` to include FPEncoding parameter (None) - Change `MemorySemantics::NONE` to `MemorySemantics::RELAXED` - Change `Decoration::UserTypeGOOGLE` to `Decoration::UserSemantic` - Update ExecutionMode NV suffixes to EXT/KHR (OutputLinesNV → OutputLinesEXT, etc.) - Add aliases for DemoteToHelperInvocationEXT and IsHelperInvocationEXT - Update ImageOperands and MemorySemantics KHR constants (now unified) - Add stubs for new operand kinds (RawAccessChainOperands, CooperativeMatrixReduce, etc.) - Update renamed ops (FinalizeNodePayloadsAMDX → EnqueueNodePayloadsAMDX, etc.) - Add catch-all for new ops in spirv_type_constraints
Improve spirv-val error messages by looking up source locations from SrcLocDecoration annotations: - Add validation_err.rs module to parse validation errors and look up source spans from the SPIR-V module's custom SrcLocDecoration - Strip SrcLocDecoration and ZombieDecoration before validation (these are invalid on functions) but keep the original module for span lookup - Add --preserve-debug-decorations flag to optionally keep decorations - Add SrcLocDecoration to entry point interface variables so validation errors for conflicting locations show the relevant source spans - Keep decorations in linker output (they're stripped after validation) For location conflict errors, the error message now shows: - The specific variable causing the conflict with its source location - The conflicting variable with its source location - A help message explaining why the conflict occurs (e.g., Mat4 uses 4 locations)
Update Cargo.toml feature documentation: - The use-compiled-tools and use-installed-tools features are now no-ops since we use a pure Rust implementation of spirv-tools - Keep features for backward compatibility with existing build configurations - Fix typo: rustc_cogeden_spirv → rustc_codegen_spirv Update test expectations: - explicit_overlap.stderr: Now shows rich error with source spans - mesh_shader.stderr: NV suffixes updated to EXT (OutputPrimitivesNV → OutputPrimitivesEXT, etc.) - member_ref_arg-broken: Updated line numbers and add normalizer for new error format
Avoids assembling the original module to a binary and then parsing it back. The validation error context can now use the Module directly for span lookups.
Provides helpful diagnostics when SPIR-V validation fails due to using a pointer from an invalid source (like OpCompositeExtract) with memory operations (OpLoad/OpStore) in logical addressing mode. The error now shows: - Which instruction cannot use the pointer - What produced the invalid pointer - Explanation of logical addressing mode restrictions - The relevant SPIR-V instructions for context
The error context already makes it clear this is a validation error.
Add rich error handlers with helpful hints for: - MissingInstructionCapability / MissingOperandCapability - MissingDescriptorSetDecoration / MissingBindingDecoration - InvalidBlockLayout - InvalidBuiltInType These handlers provide user-friendly rustc diagnostics with source spans (when available) and actionable hints for fixing common issues.
- Inline format args (uninlined_format_args) - Collapse nested if statements using let chains (collapsible_if) - Add backticks around SPIR-V terms in doc comments (doc_markdown) - Remove unused &self from format_multi_location_hint (unused_self) - Use map_or instead of map().unwrap_or() (map_unwrap_or)
Picks up 968069d2 which allows VulkanMemoryModel capability on Vulkan 1.1 via the SPV_KHR_vulkan_memory_model extension, matching the C++ spirv-val behavior.
Picks up d8c6037a which handles context-dependent number literals in OpConstant, encoding integer text for float types as the float value's IEEE 754 bits (matching C++ spirv-as).
Includes Fragment-only execution mode validation, OpLifetimeStart/Stop validation, group operation validation, and integer dot product validation.
Updates to SPIRV-Tools commit 90a479a2 which corrects required_spirv_version_for_extension to match the SPIR-V spec, removing incorrect version requirements for extensions like SPV_EXT_descriptor_indexing that only require SPIR-V 1.0.
Updates to SPIRV-Tools commit 38824bc8 which removes SPV_KHR_ray_tracing and SPV_KHR_ray_tracing_position_fetch from the extension version check to match C++ behavior, fixing compiletest failures on vulkan1.1 targets.
- Add ShaderViewportMaskNV, ShaderViewportIndexLayerEXT, and SPV_NV_viewport_array2 to all-builtins.rs compile flags to satisfy ShaderStereoViewNV capability dependency chain. - Add rspirv/spirv patch to difftest workspace Cargo.toml to match root workspace, fixing compilation errors from newer SPIR-V opcodes (ConditionalEntryPointINTEL, TypeUntypedPointerKHR, etc.) that only exist in the sdk-update branch.
Updates to SPIRV-Tools commit 06931582 which fixes grammar extension validation to use ANY-of semantics instead of ALL-of, matching C++ behavior. This fixes the ShaderViewportIndexLayerEXT validation error in all-builtins.rs where only SPV_NV_viewport_array2 is declared but the validator incorrectly also required SPV_EXT_shader_viewport_index_layer.
Updates to SPIRV-Tools commit 83c90d6e which fixes operand extension validation to use ANY-of semantics, matching the capability fix. This resolves the SPV_EXT_mesh_shader validation error in all-builtins.rs where decoration operands listed both NV and EXT mesh shader extensions as alternatives.
- FOrdEq/FOrdLe/FOrdGe reflexive rules disabled (wrong for NaN) - merge_idx < header_idx safety guard for RVSDG construct detection - Reverse De Morgan to simplify instead of expand - Disable Gamma distribution INTO rules (cycle with hoisting) - Disable Fma recognition (fires on every FAdd(FMul...)) - Reverse Neg distribution to factor instead of expand - Disable Dot/VecTimesScalar distribution over VecAdd
Picks up ccfe7ee7 which disables FMul associativity and factoring rules that caused exponential egraph growth (916K+ tuples) on shaders with many shared multiply chains.
Contributor
|
Tried locally (I'm compiling it through Logs |
Collaborator
Author
|
This is overriding the version of |
Contributor
|
Here is the commit where I tried to make a switch in case it is helpful: https://github.com/nazar-pc/abundance/compare/try-rust-spirv-tools |
Member
|
cargo-gpu just creates a git dependency with the specified rev. Could be that cargo doesn't respect the |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This can't merge as-is: