Represent global shader parameters explicitly in the IR (#756)

Before this change, global shader parameters were represented in the IR as just being ordinary global variables.
The only indication that a particular global represented a parameter was when it got a layotu attached to it (as part of back-end processing), and we've had a number of bugs related to layouts being dropped so that what should have been a shader parameter turned into an ordinary global variable in the output.

This change is more strongly motivated by the fact that making shader parameters look like globals means that we cannot easily reason about their value when doing IR transformations.
If we see two `load`s from the same global variable can we assume they yield the same value?
In the general case we cannot, and this means that any transformation that wants to rely on the fact that an input `Texture2D` shader parameter can't actually change over the life of the program needs to do extra work.

The fix here is to introduce a new kind of IR instruction that represents a global shader parameter directly (not a pointer to it as a global would), at which point there isn't even such a notion as a "load" from the parameter, since it represents the value directly.

In several cases logic that used to apply to global variables in case they were shader parameters (by looking for a layout) is now moved to apply to these global parameters.

The biggest source of issues in this change was that switching from pointers to plain values to represent these shader parameters stresses different cases in type legalization. I also had to deal with the case of legalization for GLSL where we actually *do* need global shader parameters that are writable (since varying output goes in the global scope), but in that case I borrowed the use of pointer-like `Out<...>` and `InOut<...>` types to represent that intent, which we were already using for function parameters representing outputs.

A few tests started failing because the changes lead to a slightly different order of code emission, which in some HLSL tests resulted in a function parameter named `s` getting emitted before a global parameter named `s`, leading to the latter getting the name `s_1` instead of `s_0`.

A few SPIR-V tests started failing because the new approach means that we no longer end up performing a load from all varying input parameters at the start of `main` and instead reference the varying inputs directly. The resulting code is more idomatic, but it differed from the baselines for those tests.

1 files changed, 77 insertions, 33 deletions

diff --git a/source/slang/emit.cpp b/source/slang/emit.cpp
index 930520892..39616b3df 100644
--- a/source/slang/emit.cpp
+++ b/source/slang/emit.cpp
@@ -2355,6 +2355,7 @@ struct EmitVisitor
         case kIROp_Var:
         case kIROp_GlobalVar:
         case kIROp_GlobalConstant:
+        case kIROp_GlobalParam:
         case kIROp_Param:
             return false;
 
@@ -5558,7 +5559,7 @@ struct EmitVisitor
 
     void emitHLSLParameterGroup(
         EmitContext*                    ctx,
-        IRGlobalVar*                    varDecl,
+        IRGlobalParam*                  varDecl,
         IRUniformParameterGroupType*    type)
     {
         if(as<IRTextureBufferType>(type))
@@ -5623,7 +5624,7 @@ struct EmitVisitor
 
     void emitGLSLParameterGroup(
         EmitContext*                    ctx,
-        IRGlobalVar*                    varDecl,
+        IRGlobalParam*                  varDecl,
         IRUniformParameterGroupType*    type)
     {
         auto varLayout = getVarLayout(ctx, varDecl);
@@ -5680,14 +5681,14 @@ struct EmitVisitor
 
         // If the underlying variable was an array (or array of arrays, etc.)
         // we need to emit all those array brackets here.
-        emitArrayBrackets(ctx, varDecl->getDataType()->getValueType());
+        emitArrayBrackets(ctx, varDecl->getDataType());
 
         emit(";\n");
     }
 
     void emitIRParameterGroup(
         EmitContext*                    ctx,
-        IRGlobalVar*                    varDecl,
+        IRGlobalParam*                  varDecl,
         IRUniformParameterGroupType*    type)
     {
         switch (ctx->shared->target)
@@ -5763,7 +5764,7 @@ struct EmitVisitor
 
     void emitIRStructuredBuffer_GLSL(
         EmitContext*                    ctx,
-        IRGlobalVar*                    varDecl,
+        IRGlobalParam*                  varDecl,
         IRHLSLStructuredBufferTypeBase* structuredBufferType)
     {
         // Shader storage buffer is an OpenGL 430 feature
@@ -5809,14 +5810,14 @@ struct EmitVisitor
         emit("} ");
 
         emit(getIRName(varDecl));
-        emitArrayBrackets(ctx, varDecl->getDataType()->getValueType());
+        emitArrayBrackets(ctx, varDecl->getDataType());
 
         emit(";\n");
     }
 
     void emitIRByteAddressBuffer_GLSL(
         EmitContext*                    ctx,
-        IRGlobalVar*                    varDecl,
+        IRGlobalParam*                  varDecl,
         IRByteAddressBufferTypeBase*    /* byteAddressBufferType */)
     {
         // TODO: A lot of this logic is copy-pasted from `emitIRStructuredBuffer_GLSL`.
@@ -5862,7 +5863,7 @@ struct EmitVisitor
         emit("} ");
 
         emit(getIRName(varDecl));
-        emitArrayBrackets(ctx, varDecl->getDataType()->getValueType());
+        emitArrayBrackets(ctx, varDecl->getDataType());
 
         emit(";\n");
     }
@@ -5894,6 +5895,63 @@ struct EmitVisitor
             Emit("}\n");
         }
 
+        // An ordinary global variable won't have a layout
+        // associated with it, since it is not a shader
+        // parameter.
+        //
+        SLANG_ASSERT(!getVarLayout(ctx, varDecl));
+        VarLayout* layout = nullptr;
+
+        // An ordinary global variable (which is not a
+        // shader parameter) may need special
+        // modifiers to indicate it as such.
+        //
+        switch (getTarget(ctx))
+        {
+        case CodeGenTarget::HLSL:
+            // HLSL requires the `static` modifier on any
+            // global variables; otherwise they are assumed
+            // to be uniforms.
+            Emit("static ");
+            break;
+
+        default:
+            break;
+        }
+
+        emitIRVarModifiers(ctx, layout, varDecl, varType);
+
+        emitIRRateQualifiers(ctx, varDecl);
+        emitIRType(ctx, varType, getIRName(varDecl));
+
+        // TODO: These shouldn't be needed for ordinary
+        // global variables.
+        //
+        emitIRSemantics(ctx, varDecl);
+        emitIRLayoutSemantics(ctx, varDecl);
+
+        if (varDecl->getFirstBlock())
+        {
+            Emit(" = ");
+            emit(initFuncName);
+            Emit("()");
+        }
+
+        emit(";\n\n");
+    }
+
+    void emitIRGlobalParam(
+        EmitContext*    ctx,
+        IRGlobalParam*  varDecl)
+    {
+        auto rawType = varDecl->getDataType();
+
+        auto varType = rawType;
+        if( auto outType = as<IROutTypeBase>(varType) )
+        {
+            varType = outType->getValueType();
+        }
+
         // When a global shader parameter represents a "parameter group"
         // (either a constant buffer or a parameter block with non-resource
         // data in it), we will prefer to emit it as an ordinary `cbuffer`
@@ -5985,26 +6043,11 @@ struct EmitVisitor
 
         // Need to emit appropriate modifiers here.
 
+        // We expect/require all shader parameters to
+        // have some kind of layout information associted with them.
+        //
         auto layout = getVarLayout(ctx, varDecl);
-
-        if (!layout)
-        {
-            // A global variable without a layout is just an
-            // ordinary global variable, and may need special
-            // modifiers to indicate it as such.
-            switch (getTarget(ctx))
-            {
-            case CodeGenTarget::HLSL:
-                // HLSL requires the `static` modifier on any
-                // global variables; otherwise they are assumed
-                // to be uniforms.
-                Emit("static ");
-                break;
-
-            default:
-                break;
-            }
-        }
+        SLANG_ASSERT(layout);
 
         emitIRVarModifiers(ctx, layout, varDecl, varType);
 
@@ -6015,16 +6058,13 @@ struct EmitVisitor
 
         emitIRLayoutSemantics(ctx, varDecl);
 
-        if (varDecl->getFirstBlock())
-        {
-            Emit(" = ");
-            emit(initFuncName);
-            Emit("()");
-        }
+        // A shader parameter cannot have an initializer,
+        // so we do need to consider emitting one here.
 
         emit(";\n\n");
     }
 
+
     void emitIRGlobalConstantInitializer(
         EmitContext*        ctx,
         IRGlobalConstant*   valDecl)
@@ -6098,6 +6138,10 @@ struct EmitVisitor
             emitIRGlobalVar(ctx, (IRGlobalVar*) inst);
             break;
 
+        case kIROp_GlobalParam:
+            emitIRGlobalParam(ctx, (IRGlobalParam*) inst);
+            break;
+
         case kIROp_GlobalConstant:
             emitIRGlobalConstant(ctx, (IRGlobalConstant*) inst);
             break;