From 9f3cdb61aae3dccffbfe9affc0db85305f765864 Mon Sep 17 00:00:00 2001 From: Evan Mark Hopkins <85699459+evahop@users.noreply.github.com> Date: Fri, 29 Sep 2023 06:26:23 -0400 Subject: [PATCH] [msl-out] Make varyings' struct members unique (#2521) --- src/back/msl/writer.rs | 51 +++++++++++++++++----------------- tests/in/msl-varyings.wgsl | 23 +++++++++++++++ tests/out/msl/msl-varyings.msl | 50 +++++++++++++++++++++++++++++++++ tests/snapshots.rs | 1 + 4 files changed, 99 insertions(+), 26 deletions(-) create mode 100644 tests/in/msl-varyings.wgsl create mode 100644 tests/out/msl/msl-varyings.msl diff --git a/src/back/msl/writer.rs b/src/back/msl/writer.rs index 67ab887285..77231a286d 100644 --- a/src/back/msl/writer.rs +++ b/src/back/msl/writer.rs @@ -3689,6 +3689,15 @@ impl Writer { } }; + // Since `Namer.reset` wasn't expecting struct members to be + // suddenly injected into another namespace like this, + // `self.names` doesn't keep them distinct from other variables. + // Generate fresh names for these arguments, and remember the + // mapping. + let mut flattened_member_names = FastHashMap::default(); + // Varyings' members get their own namespace + let mut varyings_namer = crate::proc::Namer::default(); + // List all the Naga `EntryPoint`'s `Function`'s arguments, // flattening structs into their members. In Metal, we will pass // each of these values to the entry point as a separate argument— @@ -3704,6 +3713,14 @@ impl Writer { member.ty, member.binding.as_ref(), )); + let name_key = NameKey::StructMember(arg.ty, member_index); + let name = match member.binding { + Some(crate::Binding::Location { .. }) => { + varyings_namer.call(&self.names[&name_key]) + } + _ => self.namer.call(&self.names[&name_key]), + }; + flattened_member_names.insert(name_key, name); } } _ => flattened_arguments.push(( @@ -3727,7 +3744,10 @@ impl Writer { _ => continue, }; has_varyings = true; - let name = &self.names[name_key]; + let name = match *name_key { + NameKey::StructMember(..) => &flattened_member_names[name_key], + _ => &self.names[name_key], + }; let ty_name = TypeContext { handle: ty, gctx: module.to_ctx(), @@ -3840,27 +3860,14 @@ impl Writer { // Then pass the remaining arguments not included in the varyings // struct. - // - // Since `Namer.reset` wasn't expecting struct members to be - // suddenly injected into the normal namespace like this, - // `self.names` doesn't keep them distinct from other variables. - // Generate fresh names for these arguments, and remember the - // mapping. - let mut flattened_member_names = FastHashMap::default(); for &(ref name_key, ty, binding) in flattened_arguments.iter() { let binding = match binding { Some(binding @ &crate::Binding::BuiltIn { .. }) => binding, _ => continue, }; - let name = if let NameKey::StructMember(ty, index) = *name_key { - // We should always insert a fresh entry here, but use - // `or_insert` to get a reference to the `String` we just - // inserted. - flattened_member_names - .entry(NameKey::StructMember(ty, index)) - .or_insert_with(|| self.namer.call(&self.names[name_key])) - } else { - &self.names[name_key] + let name = match *name_key { + NameKey::StructMember(..) => &flattened_member_names[name_key], + _ => &self.names[name_key], }; if binding == &crate::Binding::BuiltIn(crate::BuiltIn::LocalInvocationId) { @@ -4057,15 +4064,7 @@ impl Writer { )?; for (member_index, member) in members.iter().enumerate() { let key = NameKey::StructMember(arg.ty, member_index as u32); - // If it's not in the varying struct, then we should - // have passed it as its own argument and assigned - // it a new name. - let name = match member.binding { - Some(crate::Binding::BuiltIn { .. }) => { - &flattened_member_names[&key] - } - _ => &self.names[&key], - }; + let name = &flattened_member_names[&key]; if member_index != 0 { write!(self.out, ", ")?; } diff --git a/tests/in/msl-varyings.wgsl b/tests/in/msl-varyings.wgsl new file mode 100644 index 0000000000..21c6184bf4 --- /dev/null +++ b/tests/in/msl-varyings.wgsl @@ -0,0 +1,23 @@ +struct Vertex { + @location(0) position: vec2f +} + +struct NoteInstance { + @location(1) position: vec2f +} + +struct VertexOutput { + @builtin(position) position: vec4f +} + +@vertex +fn vs_main(vertex: Vertex, note: NoteInstance) -> VertexOutput { + var out: VertexOutput; + return out; +} + +@fragment +fn fs_main(in: VertexOutput, note: NoteInstance) -> @location(0) vec4f { + let position = vec3(1f); + return in.position; +} diff --git a/tests/out/msl/msl-varyings.msl b/tests/out/msl/msl-varyings.msl new file mode 100644 index 0000000000..62d635249a --- /dev/null +++ b/tests/out/msl/msl-varyings.msl @@ -0,0 +1,50 @@ +// language: metal2.0 +#include +#include + +using metal::uint; + +struct Vertex { + metal::float2 position; +}; +struct NoteInstance { + metal::float2 position; +}; +struct VertexOutput { + metal::float4 position; +}; + +struct vs_mainInput { + metal::float2 position [[attribute(0)]]; + metal::float2 position_1 [[attribute(1)]]; +}; +struct vs_mainOutput { + metal::float4 position [[position]]; +}; +vertex vs_mainOutput vs_main( + vs_mainInput varyings [[stage_in]] +) { + const Vertex vertex_ = { varyings.position }; + const NoteInstance note = { varyings.position_1 }; + VertexOutput out = {}; + VertexOutput _e3 = out; + const auto _tmp = _e3; + return vs_mainOutput { _tmp.position }; +} + + +struct fs_mainInput { + metal::float2 position [[user(loc1), center_perspective]]; +}; +struct fs_mainOutput { + metal::float4 member_1 [[color(0)]]; +}; +fragment fs_mainOutput fs_main( + fs_mainInput varyings_1 [[stage_in]] +, metal::float4 position [[position]] +) { + const VertexOutput in = { position }; + const NoteInstance note_1 = { varyings_1.position }; + metal::float3 position_1 = metal::float3(1.0); + return fs_mainOutput { in.position }; +} diff --git a/tests/snapshots.rs b/tests/snapshots.rs index 1a6159e3a8..495bea9598 100644 --- a/tests/snapshots.rs +++ b/tests/snapshots.rs @@ -776,6 +776,7 @@ fn convert_wgsl() { "constructors", Targets::SPIRV | Targets::METAL | Targets::GLSL | Targets::HLSL | Targets::WGSL, ), + ("msl-varyings", Targets::METAL), ]; for &(name, targets) in inputs.iter() {