Merge pull request #505 from KhronosGroup/msl-array-mrt

Flatten MSL MRT outputs when used as an array.

reference/opt/shaders-msl/frag/mrt-array.frag

@@ -0,0 +1,43 @@
+#pragma clang diagnostic ignored "-Wmissing-prototypes"
+
+#include <metal_stdlib>
+#include <simd/simd.h>
+
+using namespace metal;
+
+struct main0_in
+{
+    float4 vB [[user(locn1)]];
+    float4 vA [[user(locn0)]];
+};
+
+struct main0_out
+{
+    float4 FragColor_0 [[color(0)]];
+    float4 FragColor_1 [[color(1)]];
+    float4 FragColor_2 [[color(2)]];
+    float4 FragColor_3 [[color(3)]];
+};
+
+// Implementation of the GLSL mod() function, which is slightly different than Metal fmod()
+template<typename Tx, typename Ty>
+Tx mod(Tx x, Ty y)
+{
+    return x - y * floor(x / y);
+}
+
+fragment main0_out main0(main0_in in [[stage_in]])
+{
+    main0_out out = {};
+    float4 FragColor[4];
+    FragColor[0] = mod(in.vA, in.vB);
+    FragColor[1] = in.vA + in.vB;
+    FragColor[2] = in.vA - in.vB;
+    FragColor[3] = in.vA * in.vB;
+    out.FragColor_0 = FragColor[0];
+    out.FragColor_1 = FragColor[1];
+    out.FragColor_2 = FragColor[2];
+    out.FragColor_3 = FragColor[3];
+    return out;
+}
+

reference/shaders-msl/frag/mrt-array.frag

@@ -0,0 +1,53 @@
+#pragma clang diagnostic ignored "-Wmissing-prototypes"
+
+#include <metal_stdlib>
+#include <simd/simd.h>
+
+using namespace metal;
+
+struct main0_in
+{
+    float4 vB [[user(locn1)]];
+    float4 vA [[user(locn0)]];
+};
+
+struct main0_out
+{
+    float4 FragColor_0 [[color(0)]];
+    float4 FragColor_1 [[color(1)]];
+    float4 FragColor_2 [[color(2)]];
+    float4 FragColor_3 [[color(3)]];
+};
+
+// Implementation of the GLSL mod() function, which is slightly different than Metal fmod()
+template<typename Tx, typename Ty>
+Tx mod(Tx x, Ty y)
+{
+    return x - y * floor(x / y);
+}
+
+void write_deeper_in_function(thread float4 (&FragColor)[4], thread float4& vA, thread float4& vB)
+{
+    FragColor[3] = vA * vB;
+}
+
+void write_in_function(thread float4 (&FragColor)[4], thread float4& vA, thread float4& vB)
+{
+    FragColor[2] = vA - vB;
+    write_deeper_in_function(FragColor, vA, vB);
+}
+
+fragment main0_out main0(main0_in in [[stage_in]])
+{
+    main0_out out = {};
+    float4 FragColor[4];
+    FragColor[0] = mod(in.vA, in.vB);
+    FragColor[1] = in.vA + in.vB;
+    write_in_function(FragColor, in.vA, in.vB);
+    out.FragColor_0 = FragColor[0];
+    out.FragColor_1 = FragColor[1];
+    out.FragColor_2 = FragColor[2];
+    out.FragColor_3 = FragColor[3];
+    return out;
+}
+

shaders-msl/frag/mrt-array.frag

@@ -0,0 +1,24 @@
+#version 310 es
+precision mediump float;
+
+layout(location = 0) out vec4 FragColor[4];
+layout(location = 0) in vec4 vA;
+layout(location = 1) in vec4 vB;
+
+void write_deeper_in_function()
+{
+	FragColor[3] = vA * vB;
+}
+
+void write_in_function()
+{
+	FragColor[2] = vA - vB;
+	write_deeper_in_function();
+}
+
+void main()
+{
+	FragColor[0] = mod(vA, vB);
+	FragColor[1] = vA + vB;
+	write_in_function();
+}

spirv_common.hpp

@@ -99,7 +99,7 @@ class Bitset
 public:
 	Bitset() = default;
 	explicit inline Bitset(uint64_t lower_)
-		: lower(lower_)
+	    : lower(lower_)
 	{
 	}
 
@@ -739,6 +739,10 @@ struct SPIRFunction : IVariant
 		arguments.push_back({ parameter_type, id, 0u, 0u, alias_global_variable });
 	}
 
+	// Statements to be emitted when the function returns.
+	// Mostly used for lowering internal data structures onto flattened structures.
+	std::vector<std::string> fixup_statements;
+
 	bool active = false;
 	bool flush_undeclared = true;
 	bool do_combined_parameters = true;
@@ -1248,7 +1252,6 @@ static inline bool type_is_floating_point(const SPIRType &type)
 {
 	return type.basetype == SPIRType::Half || type.basetype == SPIRType::Float || type.basetype == SPIRType::Double;
 }
-
 }
 
 #endif

spirv_cross.cpp

@@ -3942,7 +3942,8 @@ bool Compiler::get_common_basic_type(const SPIRType &type, SPIRType::BaseType &b
 	}
 }
 
-void Compiler::ActiveBuiltinHandler::handle_builtin(const SPIRType &type, BuiltIn builtin, const Bitset &decoration_flags)
+void Compiler::ActiveBuiltinHandler::handle_builtin(const SPIRType &type, BuiltIn builtin,
+                                                    const Bitset &decoration_flags)
 {
 	// If used, we will need to explicitly declare a new array size for these builtins.
 

spirv_glsl.cpp

@@ -883,7 +883,8 @@ uint32_t CompilerGLSL::type_to_packed_base_size(const SPIRType &type, BufferPack
 	}
 }
 
-uint32_t CompilerGLSL::type_to_packed_alignment(const SPIRType &type, const Bitset &flags, BufferPackingStandard packing)
+uint32_t CompilerGLSL::type_to_packed_alignment(const SPIRType &type, const Bitset &flags,
+                                                BufferPackingStandard packing)
 {
 	if (!type.array.empty())
 	{
@@ -971,7 +972,8 @@ uint32_t CompilerGLSL::type_to_packed_alignment(const SPIRType &type, const Bits
 	SPIRV_CROSS_THROW("Did not find suitable rule for type. Bogus decorations?");
 }
 
-uint32_t CompilerGLSL::type_to_packed_array_stride(const SPIRType &type, const Bitset &flags, BufferPackingStandard packing)
+uint32_t CompilerGLSL::type_to_packed_array_stride(const SPIRType &type, const Bitset &flags,
+                                                   BufferPackingStandard packing)
 {
 	// Array stride is equal to aligned size of the underlying type.
 	uint32_t parent = type.parent_type;
@@ -1914,11 +1916,9 @@ void CompilerGLSL::emit_declared_builtin_block(StorageClass storage, ExecutionMo
 		block_var = &var;
 	}
 
-	global_builtins = Bitset(global_builtins.get_lower() &
+	global_builtins =
-	                         ((1ull << BuiltInPosition) |
+	    Bitset(global_builtins.get_lower() & ((1ull << BuiltInPosition) | (1ull << BuiltInPointSize) |
-	                          (1ull << BuiltInPointSize) |
+	                                          (1ull << BuiltInClipDistance) | (1ull << BuiltInCullDistance)));
-	                          (1ull << BuiltInClipDistance) |
-	                          (1ull << BuiltInCullDistance)));
 
 	// Try to collect all other declared builtins.
 	if (!emitted_block)
@@ -9063,6 +9063,8 @@ void CompilerGLSL::emit_block_chain(SPIRBlock &block)
 	}
 
 	case SPIRBlock::Return:
+		for (auto &line : current_function->fixup_statements)
+			statement(line);
 		if (processing_entry_point)
 			emit_fixup();
 

spirv_msl.cpp

@@ -550,8 +550,9 @@ uint32_t CompilerMSL::add_interface_block(StorageClass storage)
 				bool is_builtin = is_member_builtin(type, mbr_idx, &builtin);
 
 				if (should_move_to_input_buffer(mbr_type_id, is_builtin, storage))
+				{
 					move_member_to_input_buffer(type, mbr_idx);
-
+				}
 				else if (!is_builtin || has_active_builtin(builtin, storage))
 				{
 					// Add a reference to the member to the interface struct.
@@ -604,38 +605,82 @@ uint32_t CompilerMSL::add_interface_block(StorageClass storage)
 			BuiltIn builtin = BuiltIn(get_decoration(p_var->self, DecorationBuiltIn));
 
 			if (should_move_to_input_buffer(type_id, is_builtin, storage))
+			{
 				move_to_input_buffer(*p_var);
-
+			}
 			else if (!is_builtin || has_active_builtin(builtin, storage))
 			{
-				// Add a reference to the variable type to the interface struct.
+				// Arrays of MRT output is not allowed in MSL, so need to handle it specially.
-				uint32_t ib_mbr_idx = uint32_t(ib_type.member_types.size());
+				if (!is_builtin && storage == StorageClassOutput && get_entry_point().model == ExecutionModelFragment &&
-				type_id = ensure_correct_builtin_type(type_id, builtin);
+				    !type.array.empty())
-				p_var->basetype = type_id;
-				ib_type.member_types.push_back(type_id);
-
-				// Give the member a name
-				string mbr_name = ensure_valid_name(to_expression(p_var->self), "m");
-				set_member_name(ib_type_id, ib_mbr_idx, mbr_name);
-
-				// Update the original variable reference to include the structure reference
-				string qual_var_name = ib_var_ref + "." + mbr_name;
-				meta[p_var->self].decoration.qualified_alias = qual_var_name;
-
-				// Copy the variable location from the original variable to the member
-				if (get_decoration_bitset(p_var->self).get(DecorationLocation))
 				{
-					uint32_t locn = get_decoration(p_var->self, DecorationLocation);
+					if (type.array.size() != 1)
-					set_member_decoration(ib_type_id, ib_mbr_idx, DecorationLocation, locn);
+						SPIRV_CROSS_THROW("Cannot emit arrays-of-arrays with MRT.");
-					mark_location_as_used_by_shader(locn, storage);
+
+					uint32_t num_mrts = type.array_size_literal.back() ? type.array.back() :
+					                                                     get<SPIRConstant>(type.array.back()).scalar();
+
+					auto *no_array_type = &type;
+					while (!no_array_type->array.empty())
+						no_array_type = &get<SPIRType>(no_array_type->parent_type);
+
+					auto &entry_func = get<SPIRFunction>(entry_point);
+					entry_func.add_local_variable(p_var->self);
+
+					for (uint32_t i = 0; i < num_mrts; i++)
+					{
+						// Add a reference to the variable type to the interface struct.
+						uint32_t ib_mbr_idx = uint32_t(ib_type.member_types.size());
+						ib_type.member_types.push_back(no_array_type->self);
+
+						// Give the member a name
+						string mbr_name = ensure_valid_name(join(to_expression(p_var->self), "_", i), "m");
+						set_member_name(ib_type_id, ib_mbr_idx, mbr_name);
+
+						// There is no qualified alias since we need to flatten the internal array on return.
+						if (get_decoration_bitset(p_var->self).get(DecorationLocation))
+						{
+							uint32_t locn = get_decoration(p_var->self, DecorationLocation) + i;
+							set_member_decoration(ib_type_id, ib_mbr_idx, DecorationLocation, locn);
+							mark_location_as_used_by_shader(locn, storage);
+						}
+
+						// Lower the internal array to flattened output when entry point returns.
+						entry_func.fixup_statements.push_back(
+						    join(ib_var_ref, ".", mbr_name, " = ", to_name(p_var->self), "[", i, "];"));
+					}
 				}
-
+				else
-				// Mark the member as builtin if needed
-				if (is_builtin)
 				{
-					set_member_decoration(ib_type_id, ib_mbr_idx, DecorationBuiltIn, builtin);
+					// Add a reference to the variable type to the interface struct.
-					if (builtin == BuiltInPosition)
+					uint32_t ib_mbr_idx = uint32_t(ib_type.member_types.size());
-						qual_pos_var_name = qual_var_name;
+					type_id = ensure_correct_builtin_type(type_id, builtin);
+					p_var->basetype = type_id;
+					ib_type.member_types.push_back(type_id);
+
+					// Give the member a name
+					string mbr_name = ensure_valid_name(to_expression(p_var->self), "m");
+					set_member_name(ib_type_id, ib_mbr_idx, mbr_name);
+
+					// Update the original variable reference to include the structure reference
+					string qual_var_name = ib_var_ref + "." + mbr_name;
+					meta[p_var->self].decoration.qualified_alias = qual_var_name;
+
+					// Copy the variable location from the original variable to the member
+					if (get_decoration_bitset(p_var->self).get(DecorationLocation))
+					{
+						uint32_t locn = get_decoration(p_var->self, DecorationLocation);
+						set_member_decoration(ib_type_id, ib_mbr_idx, DecorationLocation, locn);
+						mark_location_as_used_by_shader(locn, storage);
+					}
+
+					// Mark the member as builtin if needed
+					if (is_builtin)
+					{
+						set_member_decoration(ib_type_id, ib_mbr_idx, DecorationBuiltIn, builtin);
+						if (builtin == BuiltInPosition)
+							qual_pos_var_name = qual_var_name;
+					}
 				}
 			}
 		}
@@ -679,8 +724,8 @@ bool CompilerMSL::should_move_to_input_buffer(uint32_t type_id, bool is_builtin,
 			if (storage == StorageClassInput)
 				SPIRV_CROSS_THROW("The fragment function stage_in structure may not include a matrix or array.");
 
-			if (storage == StorageClassOutput)
+			//if (storage == StorageClassOutput)
-				SPIRV_CROSS_THROW("The fragment function output structure may not include a matrix or array.");
+			//	SPIRV_CROSS_THROW("The fragment function output structure may not include a matrix or array.");
 		}
 	}
 
@@ -2893,9 +2938,8 @@ string CompilerMSL::func_type_decl(SPIRType &type)
 		entry_type = "vertex";
 		break;
 	case ExecutionModelFragment:
-		entry_type = execution.flags.get(ExecutionModeEarlyFragmentTests) ?
+		entry_type =
-		             "fragment [[ early_fragment_tests ]]" :
+		    execution.flags.get(ExecutionModeEarlyFragmentTests) ? "fragment [[ early_fragment_tests ]]" : "fragment";
-		             "fragment";
 		break;
 	case ExecutionModelGLCompute:
 	case ExecutionModelKernel:
@@ -2928,8 +2972,8 @@ string CompilerMSL::get_argument_address_space(const SPIRVariable &argument)
 		if (type.basetype == SPIRType::Struct)
 			return (meta[type.self].decoration.decoration_flags.get(DecorationBufferBlock) &&
 			        !meta[argument.self].decoration.decoration_flags.get(DecorationNonWritable)) ?
-			       "device" :
+			           "device" :
-			       "constant";
+			           "constant";
 
 		break;