Handle more corner cases with the CFG traversal.

reference/shaders/comp/cfg.comp

@@ -0,0 +1,82 @@
+#version 310 es
+layout(local_size_x = 1, local_size_y = 1, local_size_z = 1) in;
+
+layout(binding = 0, std430) buffer SSBO
+{
+    float data;
+} _11;
+
+void test()
+{
+    float m;
+    if ((_11.data != 0.0))
+    {
+        float tmp = 10.0;
+        _11.data = tmp;
+    }
+    else
+    {
+        float tmp_1 = 15.0;
+        _11.data = tmp_1;
+    }
+    if ((_11.data != 0.0))
+    {
+        float e;
+        if ((_11.data != 5.0))
+        {
+            if ((_11.data != 6.0))
+            {
+                e = 10.0;
+            }
+        }
+        else
+        {
+            e = 20.0;
+        }
+    }
+    switch (int(_11.data))
+    {
+        case 0:
+        {
+            float tmp_2 = 20.0;
+            _11.data = tmp_2;
+            break;
+        }
+        case 1:
+        {
+            float tmp_3 = 30.0;
+            _11.data = tmp_3;
+            break;
+        }
+    }
+    float f;
+    switch (int(_11.data))
+    {
+        case 0:
+        {
+            f = 30.0;
+            break;
+        }
+        case 1:
+        {
+            f = 40.0;
+            break;
+        }
+    }
+    int i = 0;
+    float h;
+    for (; (i < 20); i = (i + 1), h = (h + 10.0))
+    {
+    }
+    _11.data = h;
+    do
+    {
+    } while ((m != 20.0));
+    _11.data = m;
+}
+
+void main()
+{
+    test();
+}
+

shaders/comp/cfg.comp

@@ -0,0 +1,91 @@
+#version 310 es
+layout(local_size_x = 1) in;
+
+layout(std430, binding = 0) buffer SSBO
+{
+	float data;
+};
+
+void test()
+{
+	// Test that variables local to a scope stay local.
+	if (data != 0.0)
+	{
+		float tmp = 10.0;
+		data = tmp;
+	}
+	else
+	{
+		float tmp = 15.0;
+		data = tmp;
+	}
+
+	// Test that variable access propagates up to dominator
+	if (data != 0.0)
+	{
+		float e;
+		if (data != 5.0)
+		{
+			if (data != 6.0)
+				e = 10.0;
+		}
+		else
+			e = 20.0;
+	}
+
+	// Test that variables local to a switch block stay local.
+	switch (int(data))
+	{
+		case 0:
+		{
+			float tmp = 20.0;
+			data = tmp;
+			break;
+		}
+
+		case 1:
+		{
+			float tmp = 30.0;
+			data = tmp;
+			break;
+		}
+	}
+
+	// Check that multibranches propagate up to dominator.
+	float f;
+	switch (int(data))
+	{
+		case 0:
+		{
+			f = 30.0;
+			break;
+		}
+
+		case 1:
+		{
+			f = 40.0;
+			break;
+		}
+	}
+
+	// Check that loops work.
+	// Interesting case here is propagating variable access from the continue block.
+	float h;
+	for (int i = 0; i < 20; i++, h += 10.0)
+		;
+	data = h;
+
+	// Do the same with do-while, gotta test all the hard cases.
+	float m;
+	do
+	{
+	} while (m != 20.0);
+	data = m;
+}
+
+void main()
+{
+	// Test that we do the CFG analysis for all functions.
+	test();
+}
+

spirv_cfg.cpp

@@ -177,4 +177,53 @@ void DominatorBuilder::add_block(uint32_t block)
 	if (block != dominator)
 		dominator = cfg.find_common_dominator(block, dominator);
 }
+
+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.
+	// 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
+	// solution.
+
+	if (!dominator)
+		return;
+
+	auto &block = cfg.get_compiler().get<SPIRBlock>(dominator);
+	auto post_order = cfg.get_visit_order(dominator);
+
+	// If we are branching to a block with a higher post-order traversal index (continue blocks), we have a problem
+	// since we cannot create sensible GLSL code for this, fallback to entry block.
+	bool back_edge_dominator = false;
+	switch (block.terminator)
+	{
+	case SPIRBlock::Direct:
+		if (cfg.get_visit_order(block.next_block) > post_order)
+			back_edge_dominator = true;
+		break;
+
+	case SPIRBlock::Select:
+		if (cfg.get_visit_order(block.true_block) > post_order)
+			back_edge_dominator = true;
+		if (cfg.get_visit_order(block.false_block) > post_order)
+			back_edge_dominator = true;
+		break;
+
+	case SPIRBlock::MultiSelect:
+		for (auto &target : block.cases)
+		{
+			if (cfg.get_visit_order(target.block) > post_order)
+				back_edge_dominator = true;
+		}
+		if (block.default_block && cfg.get_visit_order(block.default_block) > post_order)
+			back_edge_dominator = true;
+		break;
+
+	default:
+		break;
+	}
+
+	if (back_edge_dominator)
+		dominator = cfg.get_function().entry_block;
+}
 }

spirv_cfg.hpp

@@ -18,6 +18,7 @@
 #define SPIRV_CROSS_CFG_HPP
 
 #include "spirv_cross.hpp"
+#include <assert.h>
 
 namespace spirv_cross
 {
@@ -31,14 +32,26 @@ public:
 		return compiler;
 	}
 
+	const Compiler &get_compiler() const
+	{
+		return compiler;
+	}
+
+	const SPIRFunction &get_function() const
+	{
+		return func;
+	}
+
 	uint32_t get_immediate_dominator(uint32_t block) const
 	{
 		return immediate_dominators[block];
 	}
 
-	uint32_t get_post_order(uint32_t block) const
+	uint32_t get_visit_order(uint32_t block) const
 	{
-		return post_order[block];
+		int v = visit_order[block];
+		assert(v > 0);
+		return uint32_t(v);
 	}
 
 	uint32_t find_common_dominator(uint32_t a, uint32_t b) const;
@@ -73,6 +86,8 @@ public:
 		return dominator;
 	}
 
+	void lift_continue_block_dominator();
+
 private:
 	const CFG &cfg;
 	uint32_t dominator = 0;

spirv_cross.cpp

@@ -2912,6 +2912,8 @@ void Compiler::analyze_variable_scope(SPIRFunction &entry)
 		for (auto &block : blocks)
 			builder.add_block(block);
 
+		builder.lift_continue_block_dominator();
+
 		// Add it to a per-block list of variables.
 		uint32_t dominating_block = builder.get_dominator();
 		// If all blocks here are dead code, this will be 0, so the variable in question

spirv_cross.hpp

@@ -110,6 +110,7 @@ class Compiler
 {
 public:
 	friend class CFG;
+	friend class DominatorBuilder;
 
 	// The constructor takes a buffer of SPIR-V words and parses it.
 	Compiler(std::vector<uint32_t> ir);

spirv_glsl.cpp

@@ -5508,9 +5508,13 @@ bool CompilerGLSL::attempt_emit_loop_header(SPIRBlock &block, SPIRBlock::Method
 	}
 	else if (method == SPIRBlock::MergeToDirectForLoop)
 	{
-		uint32_t current_count = statement_count;
 		auto &child = get<SPIRBlock>(block.next_block);
 
+		// This block may be a dominating block, so make sure we flush undeclared variables before building the for loop header.
+		flush_undeclared_variables(child);
+
+		uint32_t current_count = statement_count;
+
 		// If we're trying to create a true for loop,
 		// we need to make sure that all opcodes before branch statement do not actually emit any code.
 		// We can then take the condition expression and create a for (; cond ; ) { body; } structure instead.