Fix the way IR "regions" store conditions (#2216)

As part of generating high-level-language code, we have a pass that
builds a data structure representing structured control-flow `Region`s
and their nesting relationship. That data structure is then used
when emitting control-flow statements for the body of a function.

There are `Region` subtypes coresponding to different kinds of control
flow constructs. Both the `IfElseRegion` and `SwitchRegion` subtypes
were implemented to store a reference to the branch condition direclty
in the `Region`. This turns out to be the root cause of the problem.

After the nested `Region` structure is constructed, we have an IR pass
that uses the region hierarchy to detect and fix problems where the
implicit "scoping" rules of SSA form are incompatible with the scoping
rules that will be in effect when those regions are emitted as
high-level-language control-flow statements.

A bug arose when one of the SSA values that required the scoping fix
was the branch condition of an `if` statement. While the IR pass did
what it was supposed to and replaced the operand to the `IRIfElse`
instruction, doing so did not change the cached condition in the
corresponding `IfElseRegion`, and thus didn't effect the way code
got emitted for the `if(...)` condition in HLSL.

The fix here is simple: the relevant `Region` subtypes now store a
pointer to the relevant control-flow instruction rather than to
the branch condition. The emit logic can thus fetch the correct
condition from the control-flow instruction at the time it emits an
`if` or `switch`.

Note: We do not need to have the same worries around the `IRIfElse`
or `IRSwitch` instructions, nor for the `IRBlock`s that the `Region`s
still store. The passes that come after the `Region`s get created are
not supposed to alter the CFG in any way, because otherwise they would
risk changing/invalidating the `Region` structure.

Similarly, this change doesn't modify the `IRInst`s refernced in the
`Case`s for a `SwitchRegion` under the assumption that these must
always be literal integer constants, and thus cannot be changed out.

Co-authored-by: jsmall-nvidia <jsmall@nvidia.com>

3 files changed, 18 insertions, 14 deletions

diff --git a/source/slang/slang-emit-c-like.cpp b/source/slang/slang-emit-c-like.cpp
index 2d30c2109..dccfde7b7 100644
--- a/source/slang/slang-emit-c-like.cpp
+++ b/source/slang/slang-emit-c-like.cpp
@@ -2508,7 +2508,7 @@ void CLikeSourceEmitter::emitRegion(Region* inRegion)
                 // instead of the current `if(condition) {} else { elseRegion }`
 
                 m_writer->emit("if(");
-                emitOperand(ifRegion->condition, getInfo(EmitOp::General));
+                emitOperand(ifRegion->getCondition(), getInfo(EmitOp::General));
                 m_writer->emit(")\n{\n");
                 m_writer->indent();
                 emitRegion(ifRegion->thenRegion);
@@ -2570,7 +2570,7 @@ void CLikeSourceEmitter::emitRegion(Region* inRegion)
 
                 // Emit the start of our statement.
                 m_writer->emit("switch(");
-                emitOperand(switchRegion->condition, getInfo(EmitOp::General));
+                emitOperand(switchRegion->getCondition(), getInfo(EmitOp::General));
                 m_writer->emit(")\n{\n");
 
                 auto defaultCase = switchRegion->defaultCase;
diff --git a/source/slang/slang-ir-restructure.cpp b/source/slang/slang-ir-restructure.cpp
index 5dfb0179a..7ab5b4417 100644
--- a/source/slang/slang-ir-restructure.cpp
+++ b/source/slang/slang-ir-restructure.cpp
@@ -270,13 +270,12 @@ namespace Slang
                     // region representing an `if` statement.
                     //
                     auto ifInst = (IRIfElse*)terminator;
-                    auto condition = ifInst->getCondition();
                     auto trueBlock = ifInst->getTrueBlock();
                     auto falseBlock = ifInst->getFalseBlock();
                     auto afterBlock = ifInst->getAfterBlock();
 
 
-                    RefPtr<IfRegion> ifRegion = new IfRegion(parentRegion, condition);
+                    RefPtr<IfRegion> ifRegion = new IfRegion(parentRegion, ifInst);
 
                     // The region for the "then" part of things will consist of
                     // the range of blocks `[trueBlock, afterBlock)`.
@@ -461,11 +460,10 @@ namespace Slang
                     // require something like the Relooper algorithm, though.
                     //
                     auto switchInst = (IRSwitch*) terminator;
-                    auto condition = switchInst->getCondition();
                     auto breakLabel = switchInst->getBreakLabel();
                     auto defaultLabel = switchInst->getDefaultLabel();
 
-                    RefPtr<SwitchRegion> switchRegion = new SwitchRegion(parentRegion, condition);
+                    RefPtr<SwitchRegion> switchRegion = new SwitchRegion(parentRegion, switchInst);
 
                     // A direct branch to the block after the `switch` can
                     // be emitted as a `break` statement, so we will register
diff --git a/source/slang/slang-ir-restructure.h b/source/slang/slang-ir-restructure.h
index b7a0569d5..e08dba0d3 100644
--- a/source/slang/slang-ir-restructure.h
+++ b/source/slang/slang-ir-restructure.h
@@ -3,6 +3,8 @@
 
 #include "../core/slang-basic.h"
 
+#include "slang-ir-insts.h"
+
 namespace Slang
 {
     class DiagnosticSink;
@@ -107,13 +109,15 @@ namespace Slang
     class IfRegion : public SeqRegion
     {
     public:
-        IfRegion(Region* parent, IRInst* condition)
+        IfRegion(Region* parent, IRIfElse* ifElseInst)
             : SeqRegion(Region::Flavor::If, parent)
-            , condition(condition)
+            , ifElseInst(ifElseInst)
         {}
 
-        /// The IR value that controls the conditional branch
-        IRInst* condition;
+        /// The IR `ifElse` instruction
+        IRIfElse* ifElseInst;
+
+        IRInst* getCondition() { return ifElseInst->getCondition(); }
 
         /// The region to execute if the `condition` is `true`
         RefPtr<Region> thenRegion;
@@ -182,13 +186,15 @@ namespace Slang
     class SwitchRegion : public BreakableRegion
     {
     public:
-         SwitchRegion(Region* parent, IRInst* condition)
+         SwitchRegion(Region* parent, IRSwitch* switchInst)
             : BreakableRegion(Region::Flavor::Switch, parent)
-            , condition(condition)
+            , switchInst(switchInst)
         {}
 
-        /// The IR value that controls the conditional branch
-        IRInst* condition;
+        /// The IR `switch` instruction
+        IRSwitch* switchInst;
+
+        IRInst* getCondition() { return switchInst->getCondition(); }
 
         /// A collection of `case`s that share the same code.
         class Case : public RefObject