Fix variable scope when an if or else block dominates a variable.

Just like loops, we need complicated hoisting again to make this work.

reference/opt/shaders/frag/selection-block-dominator.frag

@@ -0,0 +1,19 @@
+#version 450
+
+layout(location = 0) flat in int vIndex;
+layout(location = 0) out vec4 FragColor;
+
+void main()
+{
+    for (;;)
+    {
+        if (vIndex != 1)
+        {
+            FragColor = vec4(1.0);
+            break;
+        }
+        FragColor = vec4(10.0);
+        break;
+    }
+}
+

reference/shaders/frag/selection-block-dominator.frag

@@ -0,0 +1,20 @@
+#version 450
+
+layout(location = 0) flat in int vIndex;
+layout(location = 0) out vec4 FragColor;
+
+void main()
+{
+    int v;
+    if (vIndex != 1)
+    {
+        FragColor = vec4(1.0);
+        return;
+    }
+    else
+    {
+        v = 10;
+    }
+    FragColor = vec4(float(v));
+}
+

shaders/frag/selection-block-dominator.frag

@@ -0,0 +1,18 @@
+#version 450
+layout(location = 0) out vec4 FragColor;
+layout(location = 0) flat in int vIndex; 
+
+void main()
+{
+	int v;
+	if (vIndex != 1)
+	{
+		FragColor = vec4(1.0);
+		return;
+	}
+	else
+	{
+		v = 10;
+	}
+	FragColor = vec4(v);
+}

spirv_cfg.cpp

@@ -74,8 +74,14 @@ bool CFG::is_back_edge(uint32_t to) const
 	// We have a back edge if the visit order is set with the temporary magic value 0.
 	// Crossing edges will have already been recorded with a visit order.
 	auto itr = visit_order.find(to);
-	assert(itr != end(visit_order));
+	return itr != end(visit_order) && itr->second.get() == 0;
-	return itr->second.get() == 0;
+}
+
+bool CFG::has_visited_forward_edge(uint32_t to) const
+{
+	// If > 0, we have visited the edge already, and this is not a back edge branch.
+	auto itr = visit_order.find(to);
+	return itr != end(visit_order) && itr->second.get() > 0;
 }
 
 bool CFG::post_order_visit(uint32_t block_id)
@@ -83,8 +89,10 @@ bool CFG::post_order_visit(uint32_t block_id)
 	// If we have already branched to this block (back edge), stop recursion.
 	// If our branches are back-edges, we do not record them.
 	// We have to record crossing edges however.
-	if (visit_order[block_id].get() >= 0)
+	if (has_visited_forward_edge(block_id))
-		return !is_back_edge(block_id);
+		return true;
+	else if (is_back_edge(block_id))
+		return false;
 
 	// Block back-edges from recursively revisiting ourselves.
 	visit_order[block_id].get() = 0;
@@ -123,9 +131,39 @@ bool CFG::post_order_visit(uint32_t block_id)
 	// 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)
+	// We could use has_visited_forward_edge, but this break code-gen where the merge block is unreachable in the CFG.
-		if (post_order_visit(block.merge_block))
+	if (block.merge == SPIRBlock::MergeLoop && post_order_visit(block.merge_block))
-			add_branch(block_id, block.merge_block);
+		add_branch(block_id, block.merge_block);
+
+	// If this is a selection merge, add an implied branch to the merge target.
+	// This is needed to avoid cases where an inner branch dominates the outer branch.
+	// This can happen if one of the branches exit early, e.g.:
+	// if (cond) { ...; break; } else { var = 100 } use_var(var);
+	// We can use the variable without a Phi since there is only one possible parent here.
+	// However, in this case, we need to hoist out the inner variable to outside the branch.
+	// Use same strategy as loops.
+	if (block.merge == SPIRBlock::MergeSelection && post_order_visit(block.next_block))
+	{
+		// If there is only one preceding edge to the merge block and it's not ourselves, we need a fixup.
+		// Add a fake branch so any dominator in either the if (), or else () block, or a lone case statement
+		// will be hoisted out to outside the selection merge.
+		// If size > 1, the variable will be automatically hoisted, so we should not mess with it.
+		// Adding fake branches unconditionally breaks parameter preservation analysis,
+		// which looks at how variables are accessed through the CFG.
+		auto pred_itr = preceding_edges.find(block.next_block);
+		if (pred_itr != end(preceding_edges))
+		{
+			auto &pred = pred_itr->second;
+			if (pred.size() == 1 && *pred.begin() != block_id)
+				add_branch(block_id, block.next_block);
+		}
+		else
+		{
+			// If the merge block does not have any preceding edges, i.e. unreachable, hallucinate it.
+			// We're going to do code-gen for it, and domination analysis requires that we have at least one preceding edge.
+			add_branch(block_id, block.next_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].get() = ++visit_count;

spirv_cfg.hpp

@@ -127,6 +127,7 @@ private:
 	uint32_t visit_count = 0;
 
 	bool is_back_edge(uint32_t to) const;
+	bool has_visited_forward_edge(uint32_t to) const;
 };
 
 class DominatorBuilder