[msl-out] Make varyings' struct members unique (#2521)

src/back/msl/writer.rs

@@ -3689,6 +3689,15 @@ impl<W: Write> Writer<W> {
                 }
             };
 
+            // 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<W: Write> Writer<W> {
                                 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<W: Write> Writer<W> {
                         _ => 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<W: Write> Writer<W> {
 
             // 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<W: Write> Writer<W> {
                         )?;
                         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, ", ")?;
                             }

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;
+}

tests/out/msl/msl-varyings.msl

@@ -0,0 +1,50 @@
+// language: metal2.0
+#include <metal_stdlib>
+#include <simd/simd.h>
+
+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 };
+}

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() {