Merge pull request #243 from KhronosGroup/fix-242

Declare undefined variables (OpUndef) upfront.

reference/shaders/asm/frag/undef-variable-store.asm.frag

@@ -0,0 +1,29 @@
+#version 450
+
+layout(location = 0) out vec4 _entryPointOutput;
+
+vec4 _38;
+vec4 _47;
+
+void main()
+{
+    vec4 _27;
+    do
+    {
+        vec2 _26 = vec2(0.0);
+        if (_26.x != 0.0)
+        {
+            _27 = vec4(1.0, 0.0, 0.0, 1.0);
+            break;
+        }
+        else
+        {
+            _27 = vec4(1.0, 1.0, 0.0, 1.0);
+            break;
+        }
+        _27 = _38;
+        break;
+    } while (false);
+    _entryPointOutput = _27;
+}
+

reference/shaders/comp/cfg.comp

@@ -8,7 +8,6 @@ layout(binding = 0, std430) buffer SSBO
 
 void test()
 {
-    float m;
     if (_11.data != 0.0)
     {
         float tmp = 10.0;
@@ -68,6 +67,7 @@ void test()
     {
     }
     _11.data = h;
+    float m;
     do
     {
     } while (m != 20.0);

shaders/asm/frag/undef-variable-store.asm.frag

@@ -0,0 +1,85 @@
+; SPIR-V
+; Version: 1.0
+; Generator: Khronos Glslang Reference Front End; 1
+; Bound: 50
+; Schema: 0
+               OpCapability Shader
+          %1 = OpExtInstImport "GLSL.std.450"
+               OpMemoryModel Logical GLSL450
+               OpEntryPoint Fragment %fragmentProgram "main" %_entryPointOutput
+               OpExecutionMode %fragmentProgram OriginUpperLeft
+               OpSource HLSL 500
+               OpName %fragmentProgram "fragmentProgram"
+               OpName %_fragmentProgram_ "@fragmentProgram("
+               OpName %uv "uv"
+               OpName %_entryPointOutput "@entryPointOutput"
+               OpDecorate %_entryPointOutput Location 0
+       %void = OpTypeVoid
+          %3 = OpTypeFunction %void
+      %float = OpTypeFloat 32
+    %v4float = OpTypeVector %float 4
+          %8 = OpTypeFunction %v4float
+    %v2float = OpTypeVector %float 2
+%_ptr_Function_v2float = OpTypePointer Function %v2float
+    %float_0 = OpConstant %float 0
+         %15 = OpConstantComposite %v2float %float_0 %float_0
+       %uint = OpTypeInt 32 0
+     %uint_0 = OpConstant %uint 0
+%_ptr_Function_float = OpTypePointer Function %float
+       %bool = OpTypeBool
+    %float_1 = OpConstant %float 1
+         %26 = OpConstantComposite %v4float %float_1 %float_0 %float_0 %float_1
+         %29 = OpConstantComposite %v4float %float_1 %float_1 %float_0 %float_1
+%_ptr_Output_v4float = OpTypePointer Output %v4float
+%_entryPointOutput = OpVariable %_ptr_Output_v4float Output
+%_ptr_Function_v4float = OpTypePointer Function %v4float
+      %false = OpConstantFalse %bool
+%fragmentProgram = OpFunction %void None %3
+          %5 = OpLabel
+         %35 = OpVariable %_ptr_Function_v2float Function
+         %37 = OpVariable %_ptr_Function_v4float Function
+               OpBranch %38
+         %38 = OpLabel
+               OpLoopMerge %39 %40 None
+               OpBranch %41
+         %41 = OpLabel
+               OpStore %35 %15
+         %42 = OpAccessChain %_ptr_Function_float %35 %uint_0
+         %43 = OpLoad %float %42
+         %44 = OpFOrdNotEqual %bool %43 %float_0
+               OpSelectionMerge %45 None
+               OpBranchConditional %44 %46 %47
+         %46 = OpLabel
+               OpStore %37 %26
+               OpBranch %39
+         %47 = OpLabel
+               OpStore %37 %29
+               OpBranch %39
+         %45 = OpLabel
+         %48 = OpUndef %v4float
+               OpStore %37 %48
+               OpBranch %39
+         %40 = OpLabel
+               OpBranchConditional %false %38 %39
+         %39 = OpLabel
+         %34 = OpLoad %v4float %37
+               OpStore %_entryPointOutput %34
+               OpReturn
+               OpFunctionEnd
+%_fragmentProgram_ = OpFunction %v4float None %8
+         %10 = OpLabel
+         %uv = OpVariable %_ptr_Function_v2float Function
+               OpStore %uv %15
+         %19 = OpAccessChain %_ptr_Function_float %uv %uint_0
+         %20 = OpLoad %float %19
+         %22 = OpFOrdNotEqual %bool %20 %float_0
+               OpSelectionMerge %24 None
+               OpBranchConditional %22 %23 %28
+         %23 = OpLabel
+               OpReturnValue %26
+         %28 = OpLabel
+               OpReturnValue %29
+         %24 = OpLabel
+         %31 = OpUndef %v4float
+               OpReturnValue %31
+               OpFunctionEnd

spirv_cfg.cpp

@@ -130,6 +130,13 @@ bool CFG::post_order_visit(uint32_t block_id)
 		break;
 	}
 
+	// If this is a loop header, add an implied branch to the merge target.
+	// This is needed to avoid annoying cases with do { ... } while(false) loops often generated by inliners.
+	// To the CFG, this is linear control flow, but we risk picking the do/while scope as our dominating block.
+	// This makes sure that if we are accessing a variable outside the do/while, we choose the loop header as dominator.
+	if (block.merge == SPIRBlock::MergeLoop)
+		add_branch(block_id, block.merge_block);
+
 	// Then visit ourselves. Start counting at one, to let 0 be a magic value for testing back vs. crossing edges.
 	visit_order[block_id] = ++visit_count;
 	post_order.push_back(block_id);
@@ -181,7 +188,7 @@ void DominatorBuilder::add_block(uint32_t block)
 
 void DominatorBuilder::lift_continue_block_dominator()
 {
-	// It is possible for a continue block to be the dominator if a variable is only accessed inside the while block of a do-while loop.
+	// It is possible for a continue block to be the dominator of a variable is only accessed inside the while block of a do-while loop.
 	// We cannot safely declare variables inside a continue block, so move any variable declared
 	// in a continue block to the entry block to simplify.
 	// It makes very little sense for a continue block to ever be a dominator, so fall back to the simplest

spirv_common.hpp

@@ -729,6 +729,12 @@ struct SPIRConstant : IVariant
 		return m.columns;
 	}
 
+	SPIRConstant(uint32_t constant_type_)
+	    : constant_type(constant_type_)
+	{
+		std::memset(&m, 0, sizeof(m));
+	}
+
 	SPIRConstant(uint32_t constant_type_, const uint32_t *elements, uint32_t num_elements)
 	    : constant_type(constant_type_)
 	{

spirv_cpp.cpp

@@ -242,6 +242,8 @@ void CompilerCPP::emit_resources()
 	if (emitted)
 		statement("");
 
+	declare_undefined_values();
+
 	statement("inline void init(spirv_cross_shader& s)");
 	begin_scope();
 	statement(resource_type, "::init(s);");

spirv_cross.cpp

@@ -1650,6 +1650,14 @@ void Compiler::parse(const Instruction &instruction)
 		break;
 	}
 
+	case OpConstantNull:
+	{
+		uint32_t id = ops[1];
+		uint32_t type = ops[0];
+		set<SPIRConstant>(id, type);
+		break;
+	}
+
 	case OpSpecConstantComposite:
 	case OpConstantComposite:
 	{

spirv_glsl.cpp

@@ -1578,6 +1578,23 @@ void CompilerGLSL::emit_declared_builtin_block(StorageClass storage, ExecutionMo
 	}
 }
 
+void CompilerGLSL::declare_undefined_values()
+{
+	bool emitted = false;
+	for (auto &id : ids)
+	{
+		if (id.get_type() != TypeUndef)
+			continue;
+
+		auto &undef = id.get<SPIRUndef>();
+		statement(variable_decl(get<SPIRType>(undef.basetype), to_name(undef.self), undef.self), ";");
+		emitted = true;
+	}
+
+	if (emitted)
+		statement("");
+}
+
 void CompilerGLSL::emit_resources()
 {
 	auto &execution = get_entry_point();
@@ -1772,6 +1789,8 @@ void CompilerGLSL::emit_resources()
 
 	if (emitted)
 		statement("");
+
+	declare_undefined_values();
 }
 
 // Returns a string representation of the ID, usable as a function arg.

spirv_glsl.hpp

@@ -464,6 +464,8 @@ protected:
 
 	bool type_is_empty(const SPIRType &type);
 
+	void declare_undefined_values();
+
 private:
 	void init()
 	{

spirv_hlsl.cpp

@@ -845,6 +845,8 @@ void CompilerHLSL::emit_resources()
 	if (emitted)
 		statement("");
 
+	declare_undefined_values();
+
 	if (requires_op_fmod)
 	{
 		statement("float mod(float x, float y)");

spirv_msl.cpp

@@ -990,6 +990,8 @@ void CompilerMSL::emit_resources()
 		}
 	}
 
+	declare_undefined_values();
+
 	// Output interface blocks.
 	emit_interface_block(stage_in_var_id);
 	for (auto &nsi_var : non_stage_in_input_var_ids)