Use IR pass to eliminate phi nodes (#2226)

* Use IR pass to eliminate phi nodes

"Phi nodes" are one of the key contrivances that makes SSA (Static
Single Assignment) form work. Because SSA is so great for compiler
IRs, we kind of need to deal with phi nodes, but they also get in
the way because they don't have a direct analog in most lower-level
machine ISAs or execution models, nor in most of the high-level
languages a transpiler wants to emit. As a result a compiler like
ours needs to be able to eliminate the phi nodes from a program as
part of generating output code.

(For any clever people noting that SPIR-V supports phi nodes
directly: yes, it does. It doesn't need to and it probably *shouldn't*.
Anybody involved in the decision-making knows my reasoning, and
anybody else should feel free to ask me if they want the lecture.
Anyway...)

The basic idea of elimiating phi nodes is simple enough. We replace
each phi node with a temporary variable. Uses of the phi use values
loaded from the temporary. The operation of the phi itself
(assigning a value based on the branch taken) amounts to an assignment
into the temporary.

Previously, the Slang compiler dealt with phi nodes very late in
the process of generating code: in the middle of emitting strings
of source code in a high-level language like HLSL or GLSL. Doing the
work that late in compilation has two big drawbacks:

1. Our ability to emit clean and/or optimal code is limited because
we may not be able to make certain changes to the IR, or because we
cannot make use of additional information like a dominator tree that
might be available at other points in compilation.

2. Any other IR passes that relate to temporary variables won't be
able to see the variables that we generate for phi nodes. This could
raise issues with correctness (e.g., if we want to compute live-range
information for *all* temporary variables), or performance (we have no
way to run additional IR optimization passes after phis are eliminated).

This change addresses these problems by making the elimination of
phi nodes an explicit IR pass. Additional optimizations can easily be
run after this pass (although we'd need to be careful not to run
passes that could end up introducing new phis). The pass makes use
of the information available to it to try to produce code that will
emit to "clean" HLSL/GLSL.

The core of the pass is in `slang-ir-eliminate-phis.cpp`, and is
heavily commented, so I won't describe the approach in detail here.
There are two related issues that came up, though:

First, it turned out that our emit logic for local variables (`IRVar`
instructions) wasn't using the function we'd defined named `emitVar()`.
One worrying consequence of that oversight was that the `precise`
modifier would impact generated HLSL/GLSL for variables that turned
into SSA values (including phi nodes), but *not* for local variables
that had not been SSA'd (or that had been SSA'd and then de-SSA'd).
This change also fixes that bug; it is unclear how widespread the
impact of the original issue might be.

Second, generating explicit IR temporaries for phi nodes exposed a
pre-existing bug in the `slang-ir-restructure-scoping` pass. That pass
basically detects cases where we have an instruction `I` with a use
`U` such that the use follows the rules of SSA form ("def dominates
use," meaning `I` dominations `U`), but does not follow the more
restrictive scoping rules of high-level-language output (where a value
computed "inside" a loop is not automatically visible to code outside
the loop just because it dominates that code). That pass did not
correctly account for the case where `I` was a temporary variable.
It seems that case could not arise before now because we didn't have
any passes that would move `var`, `load`, or `store` operations out
of the basic block they started in. The fix for that pass was relatively
simple, and will make the whole thing more robust in case we add more
aggressive optimizations later.

* fixup: expected test output

1 files changed, 42 insertions, 26 deletions

diff --git a/source/slang/slang-emit.cpp b/source/slang/slang-emit.cpp
index 1b94c588b..0539f9d1d 100644
--- a/source/slang/slang-emit.cpp
+++ b/source/slang/slang-emit.cpp
@@ -10,6 +10,7 @@
 #include "slang-ir-collect-global-uniforms.h"
 #include "slang-ir-dce.h"
 #include "slang-ir-dll-import.h"
+#include "slang-ir-eliminate-phis.h"
 #include "slang-ir-entry-point-uniforms.h"
 #include "slang-ir-entry-point-raw-ptr-params.h"
 #include "slang-ir-explicit-global-context.h"
@@ -731,39 +732,54 @@ Result linkAndOptimizeIR(
     lowerBitCast(targetRequest, irModule);
     simplifyIR(irModule);
 
-    // TODO(JS): We probably want to add a pass that moves phi-node temporaries to 
-    // IR.
-    // 
-    // Currently these are added as part of emit in
-    // emitPhiVarAssignments and emitPhiVarDecls
-    // 
-    // A possible mechanism might be:
-    // 1) Find all of the parameters passed between blocks
-    // 2) Make a variable for each one of them 
-    //    This could be at the scope for the function, or more ideally a scope that is 'most appropriate' for how the parameter is passed 
-    //    ie the closest scope such that the variable is in scope across the branch.
-    // 3) Replace all uses of the parameters passed into a block (except the entry block), with the temporary
-    // 3a) Remove the parameters from the start of a block (other than the entry block)
-    // 4) For all of the branches in a function
-    // 4a) For each parameter passed in the branch, assign to the temporary
-    // 4b) Replace the branch with a branch that has no parameters
     //
-    // This should lead to an equivalent function, where the parameter passing between blocks is removed, and all the temporaries
-    // are explicit in the output.
-    // 
-    // I guess there could be a desire to combine the liveness tracking into this pass, because once a phi-temporary has been moved
-    // we have lost(?) information about liveness. That could potentially be recovered, but for the phi-temporaries, their 
-    // initial liveness is trivial, it's when the assignment takes place, at the branch point. 
-    // 
-    // If all the temporaries were marked as such, then this would be fairly trivial to recreate. 
-
-    // TODO(JS): Without a pass to make all variables (including phi ones), the liveness tracking can't track everything
+    // Downstream targets may benefit from having live-range information for
+    // local variables, and our IR currently encodes a reasonably good version
+    // of that information. At this point we will insert live-range markers
+    // for local variables, on when such markers are requested.
+    //
+    // After this point in optimization, any passes that introduce new
+    // temporary variables into the IR module should take responsibility for
+    // producing their own live-range information.
+    //
     if (codeGenContext->shouldTrackLiveness())
     {
         addLivenessTrackingToModule(irModule);
 
         dumpIRIfEnabled(codeGenContext, irModule, "LIVENESS");
+    }
+
+    // As a late step, we need to take the SSA-form IR and move things *out*
+    // of SSA form, by eliminating all "phi nodes" (block parameters) and
+    // introducing explicit temporaries instead. Doing this at the IR level
+    // means that subsequent emit logic doesn't need to contend with the
+    // complexities of blocks with parameters.
+    //
+    eliminatePhis(codeGenContext, irModule);
+#if 0
+    dumpIRIfEnabled(codeGenContext, irModule, "PHIS ELIMINATED");
+#endif
 
+    // TODO: We need to insert the logic that fixes variable scoping issues
+    // here (rather than doing it very late in the emit process), because
+    // otherwise the `applyGLSLLiveness()` operation below wouldn't be
+    // able to see the live-range information that pass would need to add.
+    // For now we are avoiding that problem by simply *not* emitting live-range
+    // information when we fix variable scoping later on.
+
+    // Depending on the target, certain things that were represented as
+    // single IR instructions will need to be emitted with the help of
+    // function declaratons in output high-level code.
+    //
+    // One example of this is the live-range information, which needs
+    // to be output to GLSL code that uses a glslang extension for
+    // supporting function declarations that map directly to SPIR-V opcodes.
+    //
+    // We execute a pass here to transform any live-range instructions
+    // in the module into function calls, for the targets that require it.
+    //
+    if (codeGenContext->shouldTrackLiveness())
+    {
         if (isKhronosTarget(targetRequest))
         {
             applyGLSLLiveness(irModule);