Runtime wgpu linkage via IR traversal (#120)#124
Open
schell wants to merge 5 commits into
Open
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
This PR resolves issue #120 by moving wgpu linkage generation from proc-macro compile-time codegen to a new runtime IR traversal API (wgsl_rs::linkage::wgpu). This unifies linkage for concrete and instantiated template modules, and updates examples/tests/consumers to use the new runtime analyzer instead of the previously generated linkage module.
Changes:
- Added
wgsl_rs::linkage::wgpuruntime analyzer (analyze_module/analyze_wgsl_module) plus WGSL §14.4.1-based runtime sizing and convenience helpers. - Removed compile-time linkage codegen from
wgsl-rs-macros(deletedlinkage.rs, removed per-binding buffer helpers/constants). - Updated examples and many shader-based tests to use
WGSL_MODULE.wgsl_source()and/or the new runtime analyzer API.
Reviewed changes
Copilot reviewed 32 out of 32 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
| DEVLOG.md | Documents the runtime-linkage migration and behavioral changes. |
| crates/wgsl-rs/tests/linkage_wgpu.rs | New integration tests validating analyzer shape + WGSL sizing rules. |
| crates/wgsl-rs/src/linkage/wgpu.rs | New runtime IR traversal implementation for wgpu linkage + sizing helpers. |
| crates/wgsl-rs/src/linkage/mod.rs | Exposes the new linkage::wgpu module behind linkage-wgpu. |
| crates/wgsl-rs-macros/src/uniform.rs | Removes compile-time buffer descriptor/creator emission. |
| crates/wgsl-rs-macros/src/storage.rs | Removes compile-time buffer descriptor/creator emission. |
| crates/wgsl-rs-macros/src/linkage.rs | Removes the old proc-macro linkage generator. |
| crates/wgsl-rs-macros/src/lib.rs | Stops injecting generated linkage module; updates template comments. |
| crates/wgsl-rs-macros/Cargo.toml | Keeps linkage-wgpu as a no-op feature for compatibility. |
| crates/roundtrip-tests/src/shaders/vector_arithmetic.rs | Uses WGSL_MODULE.wgsl_source() instead of linkage::shader_source(). |
| crates/roundtrip-tests/src/shaders/type_conversions.rs | Uses WGSL_MODULE.wgsl_source() instead of linkage::shader_source(). |
| crates/roundtrip-tests/src/shaders/trig.rs | Uses WGSL_MODULE.wgsl_source() instead of linkage::shader_source(). |
| crates/roundtrip-tests/src/shaders/texture_operations.rs | Migrates to runtime analyzer for layouts/entries/bind groups. |
| crates/roundtrip-tests/src/shaders/synchronization.rs | Uses WGSL_MODULE.wgsl_source() instead of linkage::shader_source(). |
| crates/roundtrip-tests/src/shaders/select_operations.rs | Uses WGSL_MODULE.wgsl_source() and runtime analyzer for workgroup size. |
| crates/roundtrip-tests/src/shaders/rounding.rs | Uses WGSL_MODULE.wgsl_source() instead of linkage::shader_source(). |
| crates/roundtrip-tests/src/shaders/packing.rs | Uses WGSL_MODULE.wgsl_source() instead of linkage::shader_source(). |
| crates/roundtrip-tests/src/shaders/modf_frexp_ldexp.rs | Uses WGSL_MODULE.wgsl_source() instead of linkage::shader_source(). |
| crates/roundtrip-tests/src/shaders/matrix_operations.rs | Uses WGSL_MODULE.wgsl_source() instead of linkage::shader_source(). |
| crates/roundtrip-tests/src/shaders/logical_operations.rs | Uses WGSL_MODULE.wgsl_source() and runtime analyzer for workgroup size. |
| crates/roundtrip-tests/src/shaders/geometric.rs | Uses WGSL_MODULE.wgsl_source() instead of linkage::shader_source(). |
| crates/roundtrip-tests/src/shaders/exponential.rs | Uses WGSL_MODULE.wgsl_source() instead of linkage::shader_source(). |
| crates/roundtrip-tests/src/shaders/derivative_operations.rs | Migrates render-pipeline wiring to runtime analyzer entry helpers. |
| crates/roundtrip-tests/src/shaders/clamping.rs | Uses WGSL_MODULE.wgsl_source() instead of linkage::shader_source(). |
| crates/roundtrip-tests/src/shaders/bitcast.rs | Uses WGSL_MODULE.wgsl_source() instead of linkage::shader_source(). |
| crates/roundtrip-tests/src/shaders/bit_manipulation.rs | Uses WGSL_MODULE.wgsl_source() instead of linkage::shader_source(). |
| crates/roundtrip-tests/src/shaders/basic_numeric.rs | Uses WGSL_MODULE.wgsl_source() instead of linkage::shader_source(). |
| crates/roundtrip-tests/src/shaders/atomic_operations.rs | Uses WGSL_MODULE.wgsl_source() instead of linkage::shader_source(). |
| crates/roundtrip-tests/src/shaders/advanced_texture_operations.rs | Updates shader module creation + entry selection to runtime analyzer. |
| crates/gpu-tests/tests/derivative_comparison.rs | Migrates render pipeline wiring to runtime analyzer entry helpers. |
| crates/fbm-example/src/main.rs | Uses runtime analyzer for buffer descriptors, layout, bind group, entries. |
| crates/example/src/main.rs | Uses runtime analyzer for buffers/layout/bind group and pipeline wiring. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+170
to
+191
| pub fn layout_descriptor<'a>( | ||
| &'a self, | ||
| extra_label: Option<&'a str>, | ||
| ) -> wgpu::BindGroupLayoutDescriptor<'a> { | ||
| // If a suffix is provided we leak a static string for the label | ||
| // so the descriptor's label type stays `'a` (matching the | ||
| // borrow on `self.entries`). For the common no-suffix case we | ||
| // borrow `self.label` directly. | ||
| match extra_label { | ||
| Some(suffix) => { | ||
| let owned: &'static str = leak_str(&format!("{}::{}", self.label, suffix)); | ||
| wgpu::BindGroupLayoutDescriptor { | ||
| label: Some(owned), | ||
| entries: &self.entries, | ||
| } | ||
| } | ||
| None => wgpu::BindGroupLayoutDescriptor { | ||
| label: Some(&self.label), | ||
| entries: &self.entries, | ||
| }, | ||
| } | ||
| } |
Comment on lines
+202
to
+204
| /// Creates a bind group with one entry per binding in declaration | ||
| /// order. The caller must pass exactly one `BindingResource` per | ||
| /// entry in `self.entries`, in the same order. |
Comment on lines
+887
to
+896
| ir::Type::Atomic { elem } => type_layout(elem, module), | ||
| ir::Type::Struct { name, type_args } => struct_layout(name, type_args, module), | ||
| ir::Type::Ptr { elem, .. } => type_layout(elem, module), | ||
| // Samplers, textures, and type parameters aren't host-shareable. | ||
| // Return a zero layout so the analyzer doesn't choke. | ||
| ir::Type::Sampler | ||
| | ir::Type::SamplerComparison | ||
| | ir::Type::Texture { .. } | ||
| | ir::Type::TextureDepth { .. } | ||
| | ir::Type::TypeParam { .. } => TypeLayout { size: 0, align: 1 }, |
Comment on lines
+977
to
+984
| fn eval_array_len(expr: &ir::Expr, module: &ir::Module) -> Option<usize> { | ||
| match expr { | ||
| ir::Expr::Lit(ir::Lit::Int { digits, .. }) => digits.parse::<usize>().ok(), | ||
| ir::Expr::Unary { | ||
| op: ir::UnOp::Neg, | ||
| expr, | ||
| } => eval_array_len(expr, module).map(|n| n.wrapping_neg()), | ||
| ir::Expr::Ident(name) => { |
Comment on lines
777
to
781
| // For template modules (those with module-level type parameters), | ||
| // emit an `instantiate` function alongside `WGSL_MODULE`. The | ||
| // function uses `wgsl_rs::linkage::Type<Is = ...>` constraints to | ||
| // enforce at compile time that every linkage variable's concrete type | ||
| // enforce at runtime that every linkage variable's concrete type | ||
| // is consistent across all entry points that use it. |
Comment on lines
449
to
+453
| let pipeline = device.create_render_pipeline(&wgpu::RenderPipelineDescriptor { | ||
| label: Some("fill_depth_pipeline"), | ||
| layout: Some(&pipeline_layout), | ||
| vertex: fill_depth_2d::linkage::vtx_main::vertex_state(&module), | ||
| vertex: wgsl_rs::linkage::wgpu::analyze_wgsl_module(&fill_depth_2d::WGSL_MODULE) | ||
| .vertex_entry("vtx_main") |
Comment on lines
+202
to
+203
| // The label is a &'static str borrowed from the linkage's module_label. | ||
| assert_eq!(desc.label, Some("triangle")); |
868e146 to
feac080
Compare
Adds a new wgsl_rs::linkage::wgpu module that walks a wgsl_rs::Module's runtime IR (wgsl-rs-ir) to produce wgpu bind group layouts, pipeline state descriptors, and buffer descriptors. This is the first half of issue #120: it does not yet replace the existing compile-time linkage codegen in wgsl-rs-macros, but introduces the new API alongside it. The new module is gated on the linkage-wgpu feature flag (same as the existing macro-level linkage codegen) and lives in crates/wgsl-rs/src/linkage/wgpu.rs, leaving the Type<Is=...> trait alone at crates/wgsl-rs/src/linkage/mod.rs. API: - analyze_wgsl_module(&Module) -> WgpuLinkage: assembles a flattened IR (with imports + template instantiations) and analyzes it. Panics on template modules. - analyze_module(&ir::Module) -> WgpuLinkage: analyzes a concrete IR module directly. Use this after Module::instantiate for templates. - WgpuLinkage: bind_groups, vertex_entries, fragment_entries, compute_entries, buffers. Helpers like bind_group(name), buffer(name), vertex_entry(name), fragment_entry(name), compute_entry(name), shader_module(device, source), shader_module_descriptor(source). - BindGroupInfo: layout_descriptor, layout, create. Entries are sorted by binding number, matching the old compile-time codegen. - EntryPointInfo: vertex_state, fragment_state. - ComputeEntryInfo: compute_pipeline_descriptor, compute_pipeline. - BufferDescriptorInfo: create_buffer, plus the underlying pre-built BufferDescriptor<'static>. Sizing follows WGSL §14.4.1 ('Alignment and Size'). Implemented inline on ir::Type rather than depending on wgsl-rs-layout, since the wgsl-rs-layout crate is oriented around Rust types and the IR is needed here. Covers scalars, vectors, matrices, fixed-size arrays (including named const lengths), runtime arrays (size 0), atomics, pointers, and user structs (looked up by name in the assembled IR). Tests in crates/wgsl-rs/tests/linkage_wgpu.rs cover: basic shape analysis, workgroup sizes, storage access modes, struct-typed uniforms, cross-module struct resolution, template/instantiate round-tripping, and direct type_byte_size calls against the WGSL §14.4.1 rules.
…rsal Phase 2 of issue #120: removes the proc-macro-side wgpu linkage generation that was added in Phase 1's runtime analyzer, and migrates all consumers to the new wgsl_rs::linkage::wgpu API. Removed: - crates/wgsl-rs-macros/src/linkage.rs (~650 lines) - The 'cfg(feature = "linkage-wgpu")' blocks in the proc-macro's uniform.rs and storage.rs that emitted per-binding X_BUFFER_DESCRIPTOR constants and create_X_buffer functions - The linkage_fragment code in lib.rs that spliced a 'pub mod linkage' into the generated module The 'linkage-wgpu' feature on wgsl-rs-macros is now a no-op (kept for backwards compatibility with downstream Cargo features). Migrated consumers: - crates/example/src/main.rs: HelloTriangle::new now calls wgsl_rs::linkage::wgpu::analyze_wgsl_module to get the bind group layout, buffer descriptors, vertex/fragment state. - crates/fbm-example/src/main.rs: same pattern for the FBM shader. - crates/gpu-tests/tests/derivative_comparison.rs: same pattern for derivative_shader and derivative_variants_shader. - crates/roundtrip-tests/src/shaders/*.rs (~20 files): shader_source callers now use module_name::WGSL_MODULE.wgsl_source() directly; compute shader workgroup_size lookups go through analyze_wgsl_module(...).compute_entry("main").unwrap().workgroup_size. DEVLOG entry added describing the migration and the new API ergonomics (slice-based BindGroupInfo::create instead of named parameters; WGSL §14.4.1 sizing instead of size_of::<T>()).
…p crash Resolves the macOS roundtrip-tests CI failure (issue #120 followup). The CI failure ("Rust cannot catch foreign exceptions") manifested after all 105 sub-tests passed, just before the summary line printed. Three consecutive CI runs on the PR failed at the same point; the failure did not reproduce on the developer's local Mac. The cause: wgpu's macOS Metal backend was apparently retaining a reference to entry point names in wgpu descriptors. The original compile-time codegen used 'static string literals (pub const ENTRY_POINT: &str = "vtx_main"), so any retained reference was to 'static memory that never went away. The runtime IR-based linkage (crates/wgsl-rs/src/linkage/wgpu.rs, added in commit 6a3590f) borrowed the entry point name from a String inside the WgpuLinkage. If wgpu Metal's deferred callback read that reference after the WgpuLinkage was dropped, the read hit freed memory and surfaced as a foreign exception, which the wgpu 29 Metal backend then propagated across the FFI boundary as a Rust abort. The fix is to make the IR's function name 'static in the common case while still supporting monomorphization (which produces runtime names): - ir::ItemFn::name: String → Cow<'static, str> - ir_emit.rs: emit Cow::Borrowed(stringify!(#ident)) for fresh functions - substitute.rs: rename produces Cow::Owned(new_name) for monomorphized instances (e.g. id → id_f32) - wgpu.rs: EntryPointInfo::name and ComputeEntryInfo::name become Cow<'static, str>; their wgpu descriptor methods now produce VertexState<'a>/FragmentState<'a>/ComputePipelineDescriptor<'a> where the entry_point: &str is &'static for fresh functions (safe to hand to FFI) and borrowed from the owned String for monomorphized ones - BufferDescriptorInfo no longer pre-builds a 'static descriptor via Box::leak; it stores usage flags and a binding_name: String, and the descriptor() method builds wgpu::BufferDescriptor<'_> borrowing from binding_name - Removed the leak_str helper entirely - BindGroupInfo::layout_descriptor's extra_label: Option<&str> arg is dropped (the suffix leaked a 'static; callers can construct their own descriptor if they want a custom label) Net effect: zero leaked memory in the linkage layer, and the 'static property matches the original compile-time codegen for the common (non-monomorphized) case.
feac080 to
ad330b9
Compare
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
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.
Resolves #120: moves wgpu linkage generation from compile-time codegen in the
wgslproc-macro to runtime IR traversal in a newwgsl_rs::linkage::wgpumodule.Two-phase plan
Phase 1: add runtime linkage (commit 1/2)
wgsl_rs::linkage::wgpumodule (gated onlinkage-wgpufeature) atcrates/wgsl-rs/src/linkage/wgpu.rs. The existingType<Is=...>trait is preserved atcrates/wgsl-rs/src/linkage/mod.rs.analyze_wgsl_module(&Module) -> WgpuLinkageandanalyze_module(&ir::Module) -> WgpuLinkage. The former flattens imports + template instantiations into a single IRModulebefore analyzing. Convenience methods onWgpuLinkage,BindGroupInfo,EntryPointInfo,ComputeEntryInfo, andBufferDescriptorInfocover the common wgpu setup flow.ir::Typerather than depending onwgsl-rs-layout. Covers scalars, vectors, matrices, fixed-size arrays (with named-const lengths), runtime arrays (size 0), atomics, pointers, and user structs (looked up by name in the assembled IR).crates/wgsl-rs/tests/linkage_wgpu.rscovering: basic shape analysis, workgroup sizes, storage access modes, struct-typed uniforms, cross-module struct resolution, template/instantiate round-tripping, and directtype_byte_sizeagainst §14.4.1.Phase 2: remove old codegen and update consumers (commit 2/2)
crates/wgsl-rs-macros/src/linkage.rs(~650 lines).cfg(feature = "linkage-wgpu")blocks inuniform.rsandstorage.rsthat emittedX_BUFFER_DESCRIPTORandcreate_X_bufferper binding. Thelinkage-wgpufeature onwgsl-rs-macrosis now a no-op (kept for backwards compatibility).crates/example/src/main.rs,crates/fbm-example/src/main.rs,crates/gpu-tests/tests/derivative_comparison.rs, and ~20 files incrates/roundtrip-tests/src/shaders/to the newanalyze_wgsl_moduleAPI.Net effect
-585 lines overall (2134 insertions, 1156 deletions across both phases).
AI disclosure
Per
AGENTS.md, this change was produced in collaboration with an AI agent. The commit author field uses the format{human} with {llm} <{email}>. A full transcript is available on request.