Merge pull request #1995 from KhronosGroup/fix-1992

Do not optimize OpCompositeInsert for hoisted temporaries.

5 files changed, 223 insertions, 0 deletions

diff --git a/reference/shaders-no-opt/asm/frag/composite-insert-hoisted-temporaries-1.asm.frag b/reference/shaders-no-opt/asm/frag/composite-insert-hoisted-temporaries-1.asm.frag
new file mode 100644
index 00000000..f01a3282
--- /dev/null
+++ b/reference/shaders-no-opt/asm/frag/composite-insert-hoisted-temporaries-1.asm.frag
@@ -0,0 +1,27 @@
+#version 450
+
+layout(binding = 0, std430) readonly buffer SSBO
+{
+    float values0[];
+} _5;
+
+layout(binding = 1, std430) readonly buffer SSBO1
+{
+    float values1[];
+} _7;
+
+layout(location = 0) out vec2 FragColor;
+
+void main()
+{
+    vec2 _27;
+    _27 = vec2(0.0);
+    vec2 _39;
+    vec2 _40;
+    vec2 _41;
+    for (int _30 = 0; _30 < 16; _39 = _27 * _27, _40 = _39, _40.x = _5.values0[_30], _41 = _40, _41.y = _7.values1[_30], _27 += _41, _30++)
+    {
+    }
+    FragColor = _27;
+}
+
diff --git a/reference/shaders-no-opt/asm/frag/composite-insert-hoisted-temporaries-2.asm.frag b/reference/shaders-no-opt/asm/frag/composite-insert-hoisted-temporaries-2.asm.frag
new file mode 100644
index 00000000..37b66f8c
--- /dev/null
+++ b/reference/shaders-no-opt/asm/frag/composite-insert-hoisted-temporaries-2.asm.frag
@@ -0,0 +1,29 @@
+#version 450
+
+layout(binding = 0, std430) readonly buffer SSBO
+{
+    float values0[];
+} _5;
+
+layout(binding = 1, std430) readonly buffer SSBO1
+{
+    float values1[];
+} _7;
+
+layout(location = 0) out vec2 FragColor;
+
+void main()
+{
+    vec2 _27;
+    _27 = vec2(0.0);
+    vec2 _42;
+    for (int _30 = 0; _30 < 16; _27 += _42, _30++)
+    {
+        vec2 _40 = _27 * _27;
+        _40.x = _5.values0[_30];
+        _42 = _40;
+        _42.y = _7.values1[_30];
+    }
+    FragColor = _27;
+}
+
diff --git a/shaders-no-opt/asm/frag/composite-insert-hoisted-temporaries-1.asm.frag b/shaders-no-opt/asm/frag/composite-insert-hoisted-temporaries-1.asm.frag
new file mode 100644
index 00000000..050a3385
--- /dev/null
+++ b/shaders-no-opt/asm/frag/composite-insert-hoisted-temporaries-1.asm.frag
@@ -0,0 +1,80 @@
+; SPIR-V
+; Version: 1.0
+; Generator: Khronos Glslang Reference Front End; 10
+; Bound: 65
+; 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 %SSBO "SSBO"
+               OpMemberName %SSBO 0 "values0"
+               OpName %_ ""
+               OpName %SSBO1 "SSBO1"
+               OpMemberName %SSBO1 0 "values1"
+               OpName %__0 ""
+               OpName %FragColor "FragColor"
+               OpDecorate %_runtimearr_float ArrayStride 4
+               OpMemberDecorate %SSBO 0 NonWritable
+               OpMemberDecorate %SSBO 0 Offset 0
+               OpDecorate %SSBO BufferBlock
+               OpDecorate %_ DescriptorSet 0
+               OpDecorate %_ Binding 0
+               OpDecorate %_runtimearr_float_0 ArrayStride 4
+               OpMemberDecorate %SSBO1 0 NonWritable
+               OpMemberDecorate %SSBO1 0 Offset 0
+               OpDecorate %SSBO1 BufferBlock
+               OpDecorate %__0 DescriptorSet 0
+               OpDecorate %__0 Binding 1
+               OpDecorate %FragColor Location 0
+       %void = OpTypeVoid
+          %3 = OpTypeFunction %void
+      %float = OpTypeFloat 32
+    %v2float = OpTypeVector %float 2
+    %float_0 = OpConstant %float 0
+         %11 = OpConstantComposite %v2float %float_0 %float_0
+        %int = OpTypeInt 32 1
+      %int_0 = OpConstant %int 0
+     %int_16 = OpConstant %int 16
+       %bool = OpTypeBool
+%_runtimearr_float = OpTypeRuntimeArray %float
+       %SSBO = OpTypeStruct %_runtimearr_float
+%_ptr_Uniform_SSBO = OpTypePointer Uniform %SSBO
+          %_ = OpVariable %_ptr_Uniform_SSBO Uniform
+%_ptr_Uniform_float = OpTypePointer Uniform %float
+%_runtimearr_float_0 = OpTypeRuntimeArray %float
+      %SSBO1 = OpTypeStruct %_runtimearr_float_0
+%_ptr_Uniform_SSBO1 = OpTypePointer Uniform %SSBO1
+        %__0 = OpVariable %_ptr_Uniform_SSBO1 Uniform
+      %int_1 = OpConstant %int 1
+%_ptr_Output_v2float = OpTypePointer Output %v2float
+  %FragColor = OpVariable %_ptr_Output_v2float Output
+       %main = OpFunction %void None %3
+          %5 = OpLabel
+               OpBranch %17
+         %17 = OpLabel
+         %61 = OpPhi %v2float %11 %5 %d %18
+         %60 = OpPhi %int %int_0 %5 %49 %18
+         %25 = OpSLessThan %bool %60 %int_16
+               OpLoopMerge %19 %18 None
+               OpBranchConditional %25 %pre18 %19
+	   %pre18 = OpLabel
+			   OpBranch %18
+         %18 = OpLabel
+         %32 = OpAccessChain %_ptr_Uniform_float %_ %int_0 %60
+         %43 = OpAccessChain %_ptr_Uniform_float %__0 %int_0 %60
+         %33 = OpLoad %float %32
+         %44 = OpLoad %float %43
+		 %a = OpFMul %v2float %61 %61
+         %b = OpCompositeInsert %v2float %33 %a 0
+         %c = OpCompositeInsert %v2float %44 %b 1
+		 %d = OpFAdd %v2float %61 %c
+         %49 = OpIAdd %int %60 %int_1
+               OpBranch %17
+         %19 = OpLabel
+               OpStore %FragColor %61
+               OpReturn
+               OpFunctionEnd
diff --git a/shaders-no-opt/asm/frag/composite-insert-hoisted-temporaries-2.asm.frag b/shaders-no-opt/asm/frag/composite-insert-hoisted-temporaries-2.asm.frag
new file mode 100644
index 00000000..14f1f6ef
--- /dev/null
+++ b/shaders-no-opt/asm/frag/composite-insert-hoisted-temporaries-2.asm.frag
@@ -0,0 +1,82 @@
+; SPIR-V
+; Version: 1.0
+; Generator: Khronos Glslang Reference Front End; 10
+; Bound: 65
+; 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 %SSBO "SSBO"
+               OpMemberName %SSBO 0 "values0"
+               OpName %_ ""
+               OpName %SSBO1 "SSBO1"
+               OpMemberName %SSBO1 0 "values1"
+               OpName %__0 ""
+               OpName %FragColor "FragColor"
+               OpDecorate %_runtimearr_float ArrayStride 4
+               OpMemberDecorate %SSBO 0 NonWritable
+               OpMemberDecorate %SSBO 0 Offset 0
+               OpDecorate %SSBO BufferBlock
+               OpDecorate %_ DescriptorSet 0
+               OpDecorate %_ Binding 0
+               OpDecorate %_runtimearr_float_0 ArrayStride 4
+               OpMemberDecorate %SSBO1 0 NonWritable
+               OpMemberDecorate %SSBO1 0 Offset 0
+               OpDecorate %SSBO1 BufferBlock
+               OpDecorate %__0 DescriptorSet 0
+               OpDecorate %__0 Binding 1
+               OpDecorate %FragColor Location 0
+       %void = OpTypeVoid
+          %3 = OpTypeFunction %void
+      %float = OpTypeFloat 32
+    %v2float = OpTypeVector %float 2
+    %float_0 = OpConstant %float 0
+         %11 = OpConstantComposite %v2float %float_0 %float_0
+        %int = OpTypeInt 32 1
+      %int_0 = OpConstant %int 0
+     %int_16 = OpConstant %int 16
+       %bool = OpTypeBool
+%_runtimearr_float = OpTypeRuntimeArray %float
+       %SSBO = OpTypeStruct %_runtimearr_float
+%_ptr_Uniform_SSBO = OpTypePointer Uniform %SSBO
+          %_ = OpVariable %_ptr_Uniform_SSBO Uniform
+%_ptr_Uniform_float = OpTypePointer Uniform %float
+%_runtimearr_float_0 = OpTypeRuntimeArray %float
+      %SSBO1 = OpTypeStruct %_runtimearr_float_0
+%_ptr_Uniform_SSBO1 = OpTypePointer Uniform %SSBO1
+        %__0 = OpVariable %_ptr_Uniform_SSBO1 Uniform
+      %int_1 = OpConstant %int 1
+%_ptr_Output_v2float = OpTypePointer Output %v2float
+  %FragColor = OpVariable %_ptr_Output_v2float Output
+       %main = OpFunction %void None %3
+          %5 = OpLabel
+               OpBranch %17
+         %17 = OpLabel
+         %61 = OpPhi %v2float %11 %5 %d %cont
+         %60 = OpPhi %int %int_0 %5 %49 %cont
+         %25 = OpSLessThan %bool %60 %int_16
+               OpLoopMerge %19 %cont None
+               OpBranchConditional %25 %pre18 %19
+	   %pre18 = OpLabel
+			   OpBranch %18
+         %18 = OpLabel
+         %32 = OpAccessChain %_ptr_Uniform_float %_ %int_0 %60
+         %43 = OpAccessChain %_ptr_Uniform_float %__0 %int_0 %60
+         %33 = OpLoad %float %32
+         %44 = OpLoad %float %43
+		 %a = OpFMul %v2float %61 %61
+         %b = OpCompositeInsert %v2float %33 %a 0
+         %c = OpCompositeInsert %v2float %44 %b 1
+		 OpBranch %cont
+		 %cont = OpLabel
+		 %d = OpFAdd %v2float %61 %c
+         %49 = OpIAdd %int %60 %int_1
+               OpBranch %17
+         %19 = OpLabel
+               OpStore %FragColor %61
+               OpReturn
+               OpFunctionEnd
diff --git a/spirv_glsl.cpp b/spirv_glsl.cpp
index 388ec21e..fb961c8f 100644
--- a/spirv_glsl.cpp
+++ b/spirv_glsl.cpp
@@ -11267,8 +11267,13 @@ void CompilerGLSL::emit_instruction(const Instruction &instruction)
 		// forcing temporaries is not going to help.
 		// This is similar for Constant and Undef inputs.
 		// The only safe thing to RMW is SPIRExpression.
+		// If the expression has already been used (i.e. used in a continue block), we have to keep using
+		// that loop variable, since we won't be able to override the expression after the fact.
+		// If the composite is hoisted, we might never be able to properly invalidate any usage
+		// of that composite in a subsequent loop iteration.
 		if (invalid_expressions.count(composite) ||
 		    block_composite_insert_overwrite.count(composite) ||
+		    hoisted_temporaries.count(id) || hoisted_temporaries.count(composite) ||
 		    maybe_get<SPIRExpression>(composite) == nullptr)
 		{
 			can_modify_in_place = false;