Merge pull request #1310 from KhronosGroup/msl-force-declare-implicit…

…-builtins

MSL: Deal with cases where builtin is implicitly needed, declared, but unused

reference/shaders-msl-no-opt/asm/frag/input-attachment-unused-frag-coord.asm.frag

@@ -0,0 +1,25 @@
+#pragma clang diagnostic ignored "-Wmissing-prototypes"
+
+#include <metal_stdlib>
+#include <simd/simd.h>
+
+using namespace metal;
+
+struct main0_out
+{
+    float4 FragColor [[color(0)]];
+};
+
+static inline __attribute__((always_inline))
+float4 load_subpasses(thread const texture2d<float> uInput, thread float4& gl_FragCoord)
+{
+    return uInput.read(uint2(gl_FragCoord.xy), 0);
+}
+
+fragment main0_out main0(texture2d<float> uSubpass0 [[texture(0)]], texture2d<float> uSubpass1 [[texture(1)]], float4 gl_FragCoord [[position]])
+{
+    main0_out out = {};
+    out.FragColor = uSubpass0.read(uint2(gl_FragCoord.xy), 0) + load_subpasses(uSubpass1, gl_FragCoord);
+    return out;
+}
+

shaders-msl-no-opt/asm/frag/input-attachment-unused-frag-coord.asm.frag

@@ -0,0 +1,74 @@
+; SPIR-V
+; Version: 1.0
+; Generator: Khronos Glslang Reference Front End; 8
+; Bound: 35
+; Schema: 0
+               OpCapability Shader
+               OpCapability InputAttachment
+          %1 = OpExtInstImport "GLSL.std.450"
+               OpMemoryModel Logical GLSL450
+               OpEntryPoint Fragment %main "main" %FragColor %gl_FragCoord
+               OpExecutionMode %main OriginUpperLeft
+               OpSource ESSL 310
+               OpName %main "main"
+               OpName %load_subpasses_IP1_ "load_subpasses(IP1;"
+               OpName %uInput "uInput"
+               OpName %FragColor "FragColor"
+               OpName %uSubpass0 "uSubpass0"
+               OpName %uSubpass1 "uSubpass1"
+               OpName %gl_FragCoord "gl_FragCoord"
+               OpDecorate %load_subpasses_IP1_ RelaxedPrecision
+               OpDecorate %uInput RelaxedPrecision
+               OpDecorate %14 RelaxedPrecision
+               OpDecorate %19 RelaxedPrecision
+               OpDecorate %FragColor RelaxedPrecision
+               OpDecorate %FragColor Location 0
+               OpDecorate %uSubpass0 RelaxedPrecision
+               OpDecorate %uSubpass0 DescriptorSet 0
+               OpDecorate %uSubpass0 Binding 0
+               OpDecorate %uSubpass0 InputAttachmentIndex 0
+               OpDecorate %25 RelaxedPrecision
+               OpDecorate %26 RelaxedPrecision
+               OpDecorate %uSubpass1 RelaxedPrecision
+               OpDecorate %uSubpass1 DescriptorSet 0
+               OpDecorate %uSubpass1 Binding 1
+               OpDecorate %uSubpass1 InputAttachmentIndex 1
+               OpDecorate %28 RelaxedPrecision
+               OpDecorate %29 RelaxedPrecision
+               OpDecorate %gl_FragCoord BuiltIn FragCoord
+       %void = OpTypeVoid
+          %3 = OpTypeFunction %void
+      %float = OpTypeFloat 32
+          %7 = OpTypeImage %float SubpassData 0 0 0 2 Unknown
+%_ptr_UniformConstant_7 = OpTypePointer UniformConstant %7
+    %v4float = OpTypeVector %float 4
+         %10 = OpTypeFunction %v4float %_ptr_UniformConstant_7
+        %int = OpTypeInt 32 1
+      %int_0 = OpConstant %int 0
+      %v2int = OpTypeVector %int 2
+         %18 = OpConstantComposite %v2int %int_0 %int_0
+%_ptr_Output_v4float = OpTypePointer Output %v4float
+  %FragColor = OpVariable %_ptr_Output_v4float Output
+  %uSubpass0 = OpVariable %_ptr_UniformConstant_7 UniformConstant
+  %uSubpass1 = OpVariable %_ptr_UniformConstant_7 UniformConstant
+%_ptr_Input_v4float = OpTypePointer Input %v4float
+%gl_FragCoord = OpVariable %_ptr_Input_v4float Input
+       %main = OpFunction %void None %3
+          %5 = OpLabel
+         %25 = OpLoad %7 %uSubpass0
+         %26 = OpImageRead %v4float %25 %18
+         %28 = OpFunctionCall %v4float %load_subpasses_IP1_ %uSubpass1
+         %29 = OpFAdd %v4float %26 %28
+         ;%32 = OpLoad %v4float %gl_FragCoord
+         ;%33 = OpVectorShuffle %v4float %32 %32 0 1 0 1
+         ;%34 = OpFAdd %v4float %29 %33
+               OpStore %FragColor %29
+               OpReturn
+               OpFunctionEnd
+%load_subpasses_IP1_ = OpFunction %v4float None %10
+     %uInput = OpFunctionParameter %_ptr_UniformConstant_7
+         %13 = OpLabel
+         %14 = OpLoad %7 %uInput
+         %19 = OpImageRead %v4float %14 %18
+               OpReturnValue %19
+               OpFunctionEnd

spirv_msl.cpp

@@ -182,6 +182,7 @@ void CompilerMSL::build_implicit_builtins()
 			if (need_sample_pos && builtin == BuiltInSampleId)
 			{
 				builtin_sample_id_id = var.self;
+				mark_implicit_builtin(StorageClassInput, BuiltInSampleId, var.self);
 				has_sample_id = true;
 			}
 
@@ -191,18 +192,22 @@ void CompilerMSL::build_implicit_builtins()
 				{
 				case BuiltInVertexIndex:
 					builtin_vertex_idx_id = var.self;
+					mark_implicit_builtin(StorageClassInput, BuiltInVertexIndex, var.self);
 					has_vertex_idx = true;
 					break;
 				case BuiltInBaseVertex:
 					builtin_base_vertex_id = var.self;
+					mark_implicit_builtin(StorageClassInput, BuiltInBaseVertex, var.self);
 					has_base_vertex = true;
 					break;
 				case BuiltInInstanceIndex:
 					builtin_instance_idx_id = var.self;
+					mark_implicit_builtin(StorageClassInput, BuiltInInstanceIndex, var.self);
 					has_instance_idx = true;
 					break;
 				case BuiltInBaseInstance:
 					builtin_base_instance_id = var.self;
+					mark_implicit_builtin(StorageClassInput, BuiltInBaseInstance, var.self);
 					has_base_instance = true;
 					break;
 				default:
@@ -216,10 +221,12 @@ void CompilerMSL::build_implicit_builtins()
 				{
 				case BuiltInInvocationId:
 					builtin_invocation_id_id = var.self;
+					mark_implicit_builtin(StorageClassInput, BuiltInInvocationId, var.self);
 					has_invocation_id = true;
 					break;
 				case BuiltInPrimitiveId:
 					builtin_primitive_id_id = var.self;
+					mark_implicit_builtin(StorageClassInput, BuiltInPrimitiveId, var.self);
 					has_primitive_id = true;
 					break;
 				default:
@@ -230,12 +237,14 @@ void CompilerMSL::build_implicit_builtins()
 			if ((need_subgroup_mask || needs_subgroup_invocation_id) && builtin == BuiltInSubgroupLocalInvocationId)
 			{
 				builtin_subgroup_invocation_id_id = var.self;
+				mark_implicit_builtin(StorageClassInput, BuiltInSubgroupLocalInvocationId, var.self);
 				has_subgroup_invocation_id = true;
 			}
 
 			if (need_subgroup_ge_mask && builtin == BuiltInSubgroupSize)
 			{
 				builtin_subgroup_size_id = var.self;
+				mark_implicit_builtin(StorageClassInput, BuiltInSubgroupSize, var.self);
 				has_subgroup_size = true;
 			}
 
@@ -246,10 +255,12 @@ void CompilerMSL::build_implicit_builtins()
 				case BuiltInInstanceIndex:
 					// The view index here is derived from the instance index.
 					builtin_instance_idx_id = var.self;
+					mark_implicit_builtin(StorageClassInput, BuiltInInstanceIndex, var.self);
 					has_instance_idx = true;
 					break;
 				case BuiltInViewIndex:
 					builtin_view_idx_id = var.self;
+					mark_implicit_builtin(StorageClassInput, BuiltInViewIndex, var.self);
 					has_view_idx = true;
 					break;
 				default:
@@ -646,7 +657,10 @@ void CompilerMSL::mark_implicit_builtin(StorageClass storage, BuiltIn builtin, u
 
 	assert(active_builtins != nullptr);
 	active_builtins->set(builtin);
-	get_entry_point().interface_variables.push_back(id);
+
+	auto &var = get_entry_point().interface_variables;
+	if (find(begin(var), end(var), VariableID(id)) == end(var))
+		var.push_back(id);
 }
 
 uint32_t CompilerMSL::build_constant_uint_array_pointer()