Merge pull request #655 from KhronosGroup/fix-643
MSL: Properly support passing parameters by value.
diff --git a/reference/opt/shaders-hlsl/asm/frag/pass-by-value.asm.frag b/reference/opt/shaders-hlsl/asm/frag/pass-by-value.asm.frag
new file mode 100644
index 0000000..4fe2a19
--- /dev/null
+++ b/reference/opt/shaders-hlsl/asm/frag/pass-by-value.asm.frag
@@ -0,0 +1,24 @@
+cbuffer registers
+{
+ float registers_foo : packoffset(c0);
+};
+
+static float FragColor;
+
+struct SPIRV_Cross_Output
+{
+ float FragColor : SV_Target0;
+};
+
+void frag_main()
+{
+ FragColor = 10.0f + registers_foo;
+}
+
+SPIRV_Cross_Output main()
+{
+ frag_main();
+ SPIRV_Cross_Output stage_output;
+ stage_output.FragColor = FragColor;
+ return stage_output;
+}
diff --git a/reference/opt/shaders-msl/asm/frag/pass-by-value.asm.frag b/reference/opt/shaders-msl/asm/frag/pass-by-value.asm.frag
new file mode 100644
index 0000000..3858f6d
--- /dev/null
+++ b/reference/opt/shaders-msl/asm/frag/pass-by-value.asm.frag
@@ -0,0 +1,22 @@
+#include <metal_stdlib>
+#include <simd/simd.h>
+
+using namespace metal;
+
+struct Registers
+{
+ float foo;
+};
+
+struct main0_out
+{
+ float FragColor [[color(0)]];
+};
+
+fragment main0_out main0(constant Registers& registers [[buffer(0)]])
+{
+ main0_out out = {};
+ out.FragColor = 10.0 + registers.foo;
+ return out;
+}
+
diff --git a/reference/opt/shaders/asm/frag/pass-by-value.asm.frag b/reference/opt/shaders/asm/frag/pass-by-value.asm.frag
new file mode 100644
index 0000000..fbbfe18
--- /dev/null
+++ b/reference/opt/shaders/asm/frag/pass-by-value.asm.frag
@@ -0,0 +1,16 @@
+#version 450
+
+struct Registers
+{
+ float foo;
+};
+
+uniform Registers registers;
+
+layout(location = 0) out float FragColor;
+
+void main()
+{
+ FragColor = 10.0 + registers.foo;
+}
+
diff --git a/reference/shaders-hlsl/asm/frag/pass-by-value.asm.frag b/reference/shaders-hlsl/asm/frag/pass-by-value.asm.frag
new file mode 100644
index 0000000..4bc02d5
--- /dev/null
+++ b/reference/shaders-hlsl/asm/frag/pass-by-value.asm.frag
@@ -0,0 +1,29 @@
+cbuffer registers
+{
+ float registers_foo : packoffset(c0);
+};
+
+static float FragColor;
+
+struct SPIRV_Cross_Output
+{
+ float FragColor : SV_Target0;
+};
+
+float add_value(float v, float w)
+{
+ return v + w;
+}
+
+void frag_main()
+{
+ FragColor = add_value(10.0f, registers_foo);
+}
+
+SPIRV_Cross_Output main()
+{
+ frag_main();
+ SPIRV_Cross_Output stage_output;
+ stage_output.FragColor = FragColor;
+ return stage_output;
+}
diff --git a/reference/shaders-msl/asm/frag/empty-struct.asm.frag b/reference/shaders-msl/asm/frag/empty-struct.asm.frag
index 366ab16..0a56f1f 100644
--- a/reference/shaders-msl/asm/frag/empty-struct.asm.frag
+++ b/reference/shaders-msl/asm/frag/empty-struct.asm.frag
@@ -15,7 +15,7 @@
return 0.0;
}
-float GetValue_1(thread const EmptyStructTest& self)
+float GetValue_1(EmptyStructTest self)
{
return 0.0;
}
diff --git a/reference/shaders-msl/asm/frag/pass-by-value.asm.frag b/reference/shaders-msl/asm/frag/pass-by-value.asm.frag
new file mode 100644
index 0000000..6ed945e
--- /dev/null
+++ b/reference/shaders-msl/asm/frag/pass-by-value.asm.frag
@@ -0,0 +1,29 @@
+#pragma clang diagnostic ignored "-Wmissing-prototypes"
+
+#include <metal_stdlib>
+#include <simd/simd.h>
+
+using namespace metal;
+
+struct Registers
+{
+ float foo;
+};
+
+struct main0_out
+{
+ float FragColor [[color(0)]];
+};
+
+float add_value(float v, float w)
+{
+ return v + w;
+}
+
+fragment main0_out main0(constant Registers& registers [[buffer(0)]])
+{
+ main0_out out = {};
+ out.FragColor = add_value(10.0, registers.foo);
+ return out;
+}
+
diff --git a/reference/shaders/asm/frag/pass-by-value.asm.frag b/reference/shaders/asm/frag/pass-by-value.asm.frag
new file mode 100644
index 0000000..35ba2e1
--- /dev/null
+++ b/reference/shaders/asm/frag/pass-by-value.asm.frag
@@ -0,0 +1,21 @@
+#version 450
+
+struct Registers
+{
+ float foo;
+};
+
+uniform Registers registers;
+
+layout(location = 0) out float FragColor;
+
+float add_value(float v, float w)
+{
+ return v + w;
+}
+
+void main()
+{
+ FragColor = add_value(10.0, registers.foo);
+}
+
diff --git a/shaders-hlsl/asm/frag/pass-by-value.asm.frag b/shaders-hlsl/asm/frag/pass-by-value.asm.frag
new file mode 100644
index 0000000..083c85d
--- /dev/null
+++ b/shaders-hlsl/asm/frag/pass-by-value.asm.frag
@@ -0,0 +1,51 @@
+; SPIR-V
+; Version: 1.0
+; Generator: Khronos Glslang Reference Front End; 6
+; Bound: 32
+; Schema: 0
+ OpCapability Shader
+ %1 = OpExtInstImport "GLSL.std.450"
+ OpMemoryModel Logical GLSL450
+ OpEntryPoint Fragment %main "main" %FragColor
+ OpExecutionMode %main OriginUpperLeft
+ OpSource GLSL 450
+ OpName %main "main"
+ OpName %add_value_f1_f1_ "add_value(f1;f1;"
+ OpName %v "v"
+ OpName %w "w"
+ OpName %FragColor "FragColor"
+ OpName %Registers "Registers"
+ OpMemberName %Registers 0 "foo"
+ OpName %registers "registers"
+ OpDecorate %FragColor Location 0
+ OpMemberDecorate %Registers 0 Offset 0
+ OpDecorate %Registers Block
+ %void = OpTypeVoid
+ %3 = OpTypeFunction %void
+ %float = OpTypeFloat 32
+%_ptr_Function_float = OpTypePointer Function %float
+ %8 = OpTypeFunction %float %float %float
+%_ptr_Output_float = OpTypePointer Output %float
+ %FragColor = OpVariable %_ptr_Output_float Output
+ %float_10 = OpConstant %float 10
+ %Registers = OpTypeStruct %float
+%_ptr_PushConstant_Registers = OpTypePointer PushConstant %Registers
+ %registers = OpVariable %_ptr_PushConstant_Registers PushConstant
+ %int = OpTypeInt 32 1
+ %int_0 = OpConstant %int 0
+%_ptr_PushConstant_float = OpTypePointer PushConstant %float
+ %main = OpFunction %void None %3
+ %5 = OpLabel
+ %29 = OpAccessChain %_ptr_PushConstant_float %registers %int_0
+ %30 = OpLoad %float %29
+ %31 = OpFunctionCall %float %add_value_f1_f1_ %float_10 %30
+ OpStore %FragColor %31
+ OpReturn
+ OpFunctionEnd
+%add_value_f1_f1_ = OpFunction %float None %8
+ %v = OpFunctionParameter %float
+ %w = OpFunctionParameter %float
+ %12 = OpLabel
+ %15 = OpFAdd %float %v %w
+ OpReturnValue %15
+ OpFunctionEnd
diff --git a/shaders-msl/asm/frag/pass-by-value.asm.frag b/shaders-msl/asm/frag/pass-by-value.asm.frag
new file mode 100644
index 0000000..083c85d
--- /dev/null
+++ b/shaders-msl/asm/frag/pass-by-value.asm.frag
@@ -0,0 +1,51 @@
+; SPIR-V
+; Version: 1.0
+; Generator: Khronos Glslang Reference Front End; 6
+; Bound: 32
+; Schema: 0
+ OpCapability Shader
+ %1 = OpExtInstImport "GLSL.std.450"
+ OpMemoryModel Logical GLSL450
+ OpEntryPoint Fragment %main "main" %FragColor
+ OpExecutionMode %main OriginUpperLeft
+ OpSource GLSL 450
+ OpName %main "main"
+ OpName %add_value_f1_f1_ "add_value(f1;f1;"
+ OpName %v "v"
+ OpName %w "w"
+ OpName %FragColor "FragColor"
+ OpName %Registers "Registers"
+ OpMemberName %Registers 0 "foo"
+ OpName %registers "registers"
+ OpDecorate %FragColor Location 0
+ OpMemberDecorate %Registers 0 Offset 0
+ OpDecorate %Registers Block
+ %void = OpTypeVoid
+ %3 = OpTypeFunction %void
+ %float = OpTypeFloat 32
+%_ptr_Function_float = OpTypePointer Function %float
+ %8 = OpTypeFunction %float %float %float
+%_ptr_Output_float = OpTypePointer Output %float
+ %FragColor = OpVariable %_ptr_Output_float Output
+ %float_10 = OpConstant %float 10
+ %Registers = OpTypeStruct %float
+%_ptr_PushConstant_Registers = OpTypePointer PushConstant %Registers
+ %registers = OpVariable %_ptr_PushConstant_Registers PushConstant
+ %int = OpTypeInt 32 1
+ %int_0 = OpConstant %int 0
+%_ptr_PushConstant_float = OpTypePointer PushConstant %float
+ %main = OpFunction %void None %3
+ %5 = OpLabel
+ %29 = OpAccessChain %_ptr_PushConstant_float %registers %int_0
+ %30 = OpLoad %float %29
+ %31 = OpFunctionCall %float %add_value_f1_f1_ %float_10 %30
+ OpStore %FragColor %31
+ OpReturn
+ OpFunctionEnd
+%add_value_f1_f1_ = OpFunction %float None %8
+ %v = OpFunctionParameter %float
+ %w = OpFunctionParameter %float
+ %12 = OpLabel
+ %15 = OpFAdd %float %v %w
+ OpReturnValue %15
+ OpFunctionEnd
diff --git a/shaders/asm/frag/pass-by-value.asm.frag b/shaders/asm/frag/pass-by-value.asm.frag
new file mode 100644
index 0000000..083c85d
--- /dev/null
+++ b/shaders/asm/frag/pass-by-value.asm.frag
@@ -0,0 +1,51 @@
+; SPIR-V
+; Version: 1.0
+; Generator: Khronos Glslang Reference Front End; 6
+; Bound: 32
+; Schema: 0
+ OpCapability Shader
+ %1 = OpExtInstImport "GLSL.std.450"
+ OpMemoryModel Logical GLSL450
+ OpEntryPoint Fragment %main "main" %FragColor
+ OpExecutionMode %main OriginUpperLeft
+ OpSource GLSL 450
+ OpName %main "main"
+ OpName %add_value_f1_f1_ "add_value(f1;f1;"
+ OpName %v "v"
+ OpName %w "w"
+ OpName %FragColor "FragColor"
+ OpName %Registers "Registers"
+ OpMemberName %Registers 0 "foo"
+ OpName %registers "registers"
+ OpDecorate %FragColor Location 0
+ OpMemberDecorate %Registers 0 Offset 0
+ OpDecorate %Registers Block
+ %void = OpTypeVoid
+ %3 = OpTypeFunction %void
+ %float = OpTypeFloat 32
+%_ptr_Function_float = OpTypePointer Function %float
+ %8 = OpTypeFunction %float %float %float
+%_ptr_Output_float = OpTypePointer Output %float
+ %FragColor = OpVariable %_ptr_Output_float Output
+ %float_10 = OpConstant %float 10
+ %Registers = OpTypeStruct %float
+%_ptr_PushConstant_Registers = OpTypePointer PushConstant %Registers
+ %registers = OpVariable %_ptr_PushConstant_Registers PushConstant
+ %int = OpTypeInt 32 1
+ %int_0 = OpConstant %int 0
+%_ptr_PushConstant_float = OpTypePointer PushConstant %float
+ %main = OpFunction %void None %3
+ %5 = OpLabel
+ %29 = OpAccessChain %_ptr_PushConstant_float %registers %int_0
+ %30 = OpLoad %float %29
+ %31 = OpFunctionCall %float %add_value_f1_f1_ %float_10 %30
+ OpStore %FragColor %31
+ OpReturn
+ OpFunctionEnd
+%add_value_f1_f1_ = OpFunction %float None %8
+ %v = OpFunctionParameter %float
+ %w = OpFunctionParameter %float
+ %12 = OpLabel
+ %15 = OpFAdd %float %v %w
+ OpReturnValue %15
+ OpFunctionEnd
diff --git a/spirv_msl.cpp b/spirv_msl.cpp
index e9a49e4..154062a 100644
--- a/spirv_msl.cpp
+++ b/spirv_msl.cpp
@@ -565,10 +565,20 @@
if (is_builtin && has_active_builtin(builtin, var.storage))
{
// Add a arg variable with the same type and decorations as the member
- uint32_t next_id = increase_bound_by(1);
- func.add_parameter(mbr_type_id, next_id, true);
- set<SPIRVariable>(next_id, mbr_type_id, StorageClassFunction);
- meta[next_id].decoration = meta[type_id].members[mbr_idx];
+ uint32_t next_ids = increase_bound_by(2);
+ uint32_t ptr_type_id = next_ids + 0;
+ uint32_t var_id = next_ids + 1;
+
+ // Make sure we have an actual pointer type,
+ // so that we will get the appropriate address space when declaring these builtins.
+ auto &ptr = set<SPIRType>(ptr_type_id, get<SPIRType>(mbr_type_id));
+ ptr.self = mbr_type_id;
+ ptr.storage = var.storage;
+ ptr.pointer = true;
+
+ func.add_parameter(mbr_type_id, var_id, true);
+ set<SPIRVariable>(var_id, ptr_type_id, StorageClassFunction);
+ meta[var_id].decoration = meta[type_id].members[mbr_idx];
}
mbr_idx++;
}
@@ -2400,8 +2410,7 @@
{
add_local_variable_name(arg.id);
- string address_space = "thread";
-
+ string address_space;
auto *var = maybe_get<SPIRVariable>(arg.id);
if (var)
{
@@ -2409,7 +2418,8 @@
address_space = get_argument_address_space(*var);
}
- decl += address_space + " ";
+ if (!address_space.empty())
+ decl += address_space + " ";
decl += argument_decl(arg);
// Manufacture automatic sampler arg for SampledImage texture
@@ -3161,6 +3171,11 @@
}
break;
+ case StorageClassFunction:
+ case StorageClassGeneric:
+ // No address space for plain values.
+ return type.pointer ? "thread" : "";
+
default:
break;
}
@@ -3374,7 +3389,8 @@
{
auto &var = get<SPIRVariable>(arg.id);
auto &type = expression_type(arg.id);
- bool constref = !arg.alias_global_variable && (!type.pointer || arg.write_count == 0);
+
+ bool constref = !arg.alias_global_variable && type.pointer && arg.write_count == 0;
bool type_is_image = type.basetype == SPIRType::Image || type.basetype == SPIRType::SampledImage ||
type.basetype == SPIRType::Sampler;
@@ -3383,27 +3399,34 @@
if (!type.array.empty() && type_is_image)
constref = true;
- // TODO: Check if this arg is an uniform pointer
- bool pointer = type.storage == StorageClassUniformConstant;
-
string decl;
if (constref)
decl += "const ";
- if (is_builtin_variable(var))
+ bool builtin = is_builtin_variable(var);
+ if (builtin)
decl += builtin_type_decl(static_cast<BuiltIn>(get_decoration(arg.id, DecorationBuiltIn)));
else
decl += type_to_glsl(type, arg.id);
- // Arrays of images and samplers are special cased.
- if (is_array(type) && !type_is_image)
+ bool opaque_handle = type.storage == StorageClassUniformConstant;
+
+ if (!builtin && !opaque_handle && !type.pointer &&
+ (type.storage == StorageClassFunction || type.storage == StorageClassGeneric))
{
+ // If the argument is a pure value and not an opaque type, we will pass by value.
+ decl += " ";
+ decl += to_expression(var.self);
+ }
+ else if (is_array(type) && !type_is_image)
+ {
+ // Arrays of images and samplers are special cased.
decl += " (&";
decl += to_expression(var.self);
decl += ")";
decl += type_to_array_glsl(type);
}
- else if (!pointer)
+ else if (!opaque_handle)
{
decl += "&";
decl += " ";
