diff options
author | Hans-Kristian Arntzen <hans-kristian.arntzen@arm.com> | 2018-08-06 17:02:35 +0300 |
---|---|---|
committer | GitHub <noreply@github.com> | 2018-08-06 17:02:35 +0300 |
commit | 1d1b5eb865a85013a5981b45757baaaa9a3b0f62 (patch) | |
tree | 951029eb94b09f4ad8906a161616a3760632299e | |
parent | de5195cbb2e814598566dd17cc4db1cefcdf68a7 (diff) | |
parent | 361fe52c9dfebf83c79d64daabf08e1111ab3fbf (diff) |
Merge pull request #655 from KhronosGroup/fix-6432018-08-07
MSL: Properly support passing parameters by value.
-rw-r--r-- | reference/opt/shaders-hlsl/asm/frag/pass-by-value.asm.frag | 24 | ||||
-rw-r--r-- | reference/opt/shaders-msl/asm/frag/pass-by-value.asm.frag | 22 | ||||
-rw-r--r-- | reference/opt/shaders/asm/frag/pass-by-value.asm.frag | 16 | ||||
-rw-r--r-- | reference/shaders-hlsl/asm/frag/pass-by-value.asm.frag | 29 | ||||
-rw-r--r-- | reference/shaders-msl/asm/frag/empty-struct.asm.frag | 2 | ||||
-rw-r--r-- | reference/shaders-msl/asm/frag/pass-by-value.asm.frag | 29 | ||||
-rw-r--r-- | reference/shaders/asm/frag/pass-by-value.asm.frag | 21 | ||||
-rw-r--r-- | shaders-hlsl/asm/frag/pass-by-value.asm.frag | 51 | ||||
-rw-r--r-- | shaders-msl/asm/frag/pass-by-value.asm.frag | 51 | ||||
-rw-r--r-- | shaders/asm/frag/pass-by-value.asm.frag | 51 | ||||
-rw-r--r-- | spirv_msl.cpp | 53 |
11 files changed, 333 insertions, 16 deletions
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 00000000..4fe2a197 --- /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 00000000..3858f6da --- /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 00000000..fbbfe18b --- /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 00000000..4bc02d5b --- /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 366ab165..0a56f1f1 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 @@ float GetValue(thread const EmptyStructTest& self) 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 00000000..6ed945ec --- /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 00000000..35ba2e15 --- /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 00000000..083c85d9 --- /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 00000000..083c85d9 --- /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 00000000..083c85d9 --- /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 e9a49e43..154062a7 100644 --- a/spirv_msl.cpp +++ b/spirv_msl.cpp @@ -565,10 +565,20 @@ void CompilerMSL::extract_global_variables_from_function(uint32_t func_id, std:: 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 @@ void CompilerMSL::emit_function_prototype(SPIRFunction &func, const Bitset &) { 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 @@ void CompilerMSL::emit_function_prototype(SPIRFunction &func, const Bitset &) 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 @@ string CompilerMSL::get_argument_address_space(const SPIRVariable &argument) } break; + case StorageClassFunction: + case StorageClassGeneric: + // No address space for plain values. + return type.pointer ? "thread" : ""; + default: break; } @@ -3374,7 +3389,8 @@ string CompilerMSL::argument_decl(const SPIRFunction::Parameter &arg) { 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 @@ string CompilerMSL::argument_decl(const SPIRFunction::Parameter &arg) 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 += " "; |