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, 95 insertions, 85 deletions

diff --git a/source/slang/ir.cpp b/source/slang/ir.cpp
index b8892cc02..0d93957c8 100644
--- a/source/slang/ir.cpp
+++ b/source/slang/ir.cpp
@@ -1966,6 +1966,18 @@ namespace Slang
         return globalConstant;
     }
 
+    IRGlobalParam* IRBuilder::createGlobalParam(
+        IRType* valueType)
+    {
+        IRGlobalParam* inst = createInst<IRGlobalParam>(
+            this,
+            kIROp_GlobalParam,
+            valueType);
+        maybeSetSourceLoc(this, inst);
+        addGlobalValue(this, inst);
+        return inst;
+    }
+
     IRWitnessTable* IRBuilder::createWitnessTable()
     {
         IRWitnessTable* witnessTable = createInst<IRWitnessTable>(
@@ -3730,6 +3742,7 @@ namespace Slang
         case kIROp_Generic:
         case kIROp_GlobalVar:
         case kIROp_GlobalConstant:
+        case kIROp_GlobalParam:
         case kIROp_StructKey:
         case kIROp_GlobalGenericParam:
         case kIROp_WitnessTable:
@@ -3800,7 +3813,7 @@ namespace Slang
     // Legalization of entry points for GLSL:
     //
 
-    IRGlobalVar* addGlobalVariable(
+    IRGlobalParam* addGlobalParam(
         IRModule*   module,
         IRType*     valueType)
     {
@@ -3812,7 +3825,7 @@ namespace Slang
 
         IRBuilder builder;
         builder.sharedBuilder = &shared;
-        return builder.createGlobalVar(valueType);
+        return builder.createGlobalParam(valueType);
     }
 
     void moveValueBefore(
@@ -4277,18 +4290,26 @@ namespace Slang
         varLayout->stage = inVarLayout->stage;
         varLayout->AddResourceInfo(kind)->index = bindingIndex;
 
-        // Simple case: just create a global variable of the matching type,
-        // and then use the value of the global as a replacement for the
-        // value of the original parameter.
+        // We are going to be creating a global parameter to replace
+        // the function parameter, but we need to handle the case
+        // where the parameter represents a varying *output* and not
+        // just an input.
+        //
+        // Our IR global shader parameters are read-only, just
+        // like our IR function parameters, and need a wrapper
+        // `Out<...>` type to represent otuputs.
         //
-        auto globalVariable = addGlobalVariable(builder->getModule(), type);
-        moveValueBefore(globalVariable, builder->getFunc());
+        bool isOutput = kind == LayoutResourceKind::VaryingOutput;
+        IRType* paramType = isOutput ? builder->getOutType(type) : type;
+
+        auto globalParam = addGlobalParam(builder->getModule(), paramType);
+        moveValueBefore(globalParam, builder->getFunc());
 
-        ScalarizedVal val = ScalarizedVal::address(globalVariable);
+        ScalarizedVal val = isOutput ? ScalarizedVal::address(globalParam) : ScalarizedVal::value(globalParam);
 
         if( systemValueInfo )
         {
-            builder->addImportDecoration(globalVariable, UnownedTerminatedStringSlice(systemValueInfo->name));
+            builder->addImportDecoration(globalParam, UnownedTerminatedStringSlice(systemValueInfo->name));
 
             if( auto fromType = systemValueInfo->requiredType )
             {
@@ -4309,11 +4330,11 @@ namespace Slang
 
             if(auto outerArrayName = systemValueInfo->outerArrayName)
             {
-                builder->addGLSLOuterArrayDecoration(globalVariable, UnownedTerminatedStringSlice(outerArrayName));
+                builder->addGLSLOuterArrayDecoration(globalParam, UnownedTerminatedStringSlice(outerArrayName));
             }
         }
 
-        builder->addLayoutDecoration(globalVariable, varLayout);
+        builder->addLayoutDecoration(globalParam, varLayout);
 
         return val;
     }
@@ -4865,46 +4886,22 @@ namespace Slang
         auto builder = context->getBuilder();
         auto paramType = pp->getDataType();
 
-        if(auto paramPtrType = as<IROutTypeBase>(paramType) )
-        {
-            // This is either an `out` or `in out` parameter.
-            // We want to treat `out` parameters the same
-            // as `in out` for our purposes, since there are
-            // no pure `out` parameters defined for the ray
-            // tracing stages.
-
-            // Unlike the default legalization strategy for
-            // `out` and `in out` entry point parameters,
-            // we will not introduce an intermediate temporary.
-            //
-            // Instead we will simply create a global variable
-            // and replace uses of the parameter with uses
-            // of that global variable.
-
-            auto valueType = paramPtrType->getValueType();
-
-            auto globalVariable = addGlobalVariable(builder->getModule(), valueType);
-            builder->addLayoutDecoration(globalVariable, paramLayout);
-            moveValueBefore(globalVariable, builder->getFunc());
-
-            pp->replaceUsesWith(globalVariable);
-        }
-        else
-        {
-            // This is the `in` parameter case, so that the parameter
-            // was not a pointer. We will allocate a global variable
-            // to represent the parameter, and then perform a load
-            // form it at the start of the function.
-            //
-            auto valueType = paramType;
-            auto globalVariable = addGlobalVariable(builder->getModule(), valueType);
-            builder->addLayoutDecoration(globalVariable, paramLayout);
-            moveValueBefore(globalVariable, builder->getFunc());
-
-            auto irLoad = builder->emitLoad(globalVariable);
-            pp->replaceUsesWith(irLoad);
-        }
-
+        // The parameter might be either an `in` parameter,
+        // or an `out` or `in out` parameter, and in those
+        // latter cases its IR-level type will include a
+        // wrapping "pointer-like" type (e.g., `Out<Float>`
+        // instead of just `Float`).
+        //
+        // Because global shader parameters are read-only
+        // in the same way function types are, we can take
+        // care of that detail here just by allocating a
+        // global shader parameter with exactly the type
+        // of the original function parameter.
+        //
+        auto globalParam = addGlobalParam(builder->getModule(), paramType);
+        builder->addLayoutDecoration(globalParam, paramLayout);
+        moveValueBefore(globalParam, builder->getFunc());
+        pp->replaceUsesWith(globalParam);
     }
 
     void legalizeEntryPointParameterForGLSL(
@@ -5629,6 +5626,7 @@ namespace Slang
         case kIROp_Generic:
         case kIROp_GlobalVar:
         case kIROp_GlobalConstant:
+        case kIROp_GlobalParam:
         case kIROp_StructKey:
         case kIROp_GlobalGenericParam:
         case kIROp_WitnessTable:
@@ -5779,21 +5777,6 @@ namespace Slang
 
         registerClonedValue(context, clonedVar, originalValues);
 
-#if 0
-        auto mangledName = originalVar->mangledName;
-        clonedVar->mangledName = mangledName;
-#endif
-
-        if(auto linkage = originalVar->findDecoration<IRLinkageDecoration>())
-        {
-            auto mangledName = String(linkage->getMangledName());
-            VarLayout* layout = nullptr;
-            if (context->globalVarLayouts.TryGetValue(mangledName, layout))
-            {
-                builder->addLayoutDecoration(clonedVar, layout);
-            }
-        }
-
         // Clone any code in the body of the variable, since this
         // represents the initializer.
         cloneGlobalValueWithCodeCommon(
@@ -5824,25 +5807,6 @@ namespace Slang
         return clonedVal;
     }
 
-    IRGeneric* cloneGenericImpl(
-        IRSpecContextBase*              context,
-        IRBuilder*                      builder,
-        IRGeneric*                      originalVal,
-        IROriginalValuesForClone const& originalValues)
-    {
-        auto clonedVal = builder->emitGeneric();
-        registerClonedValue(context, clonedVal, originalValues);
-
-        // Clone any code in the body of the generic, since this
-        // computes its result value.
-        cloneGlobalValueWithCodeCommon(
-            context,
-            clonedVal,
-            originalVal);
-
-        return clonedVal;
-    }
-
     void cloneSimpleGlobalValueImpl(
         IRSpecContextBase*              context,
         IRInst*                         originalInst,
@@ -5865,6 +5829,48 @@ namespace Slang
         }
     }
 
+    IRGlobalParam* cloneGlobalParamImpl(
+        IRSpecContextBase*              context,
+        IRBuilder*                      builder,
+        IRGlobalParam*                  originalVal,
+        IROriginalValuesForClone const& originalValues)
+    {
+        auto clonedVal = builder->createGlobalParam(
+            cloneType(context, originalVal->getFullType()));
+        cloneSimpleGlobalValueImpl(context, originalVal, originalValues, clonedVal);
+
+        if(auto linkage = originalVal->findDecoration<IRLinkageDecoration>())
+        {
+            auto mangledName = String(linkage->getMangledName());
+            VarLayout* layout = nullptr;
+            if (context->globalVarLayouts.TryGetValue(mangledName, layout))
+            {
+                builder->addLayoutDecoration(clonedVal, layout);
+            }
+        }
+
+        return clonedVal;
+    }
+
+    IRGeneric* cloneGenericImpl(
+        IRSpecContextBase*              context,
+        IRBuilder*                      builder,
+        IRGeneric*                      originalVal,
+        IROriginalValuesForClone const& originalValues)
+    {
+        auto clonedVal = builder->emitGeneric();
+        registerClonedValue(context, clonedVal, originalValues);
+
+        // Clone any code in the body of the generic, since this
+        // computes its result value.
+        cloneGlobalValueWithCodeCommon(
+            context,
+            clonedVal,
+            originalVal);
+
+        return clonedVal;
+    }
+
     IRStructKey* cloneStructKeyImpl(
         IRSpecContextBase*              context,
         IRBuilder*                      builder,
@@ -6254,6 +6260,7 @@ namespace Slang
 
         case kIROp_StructType:
         case kIROp_GlobalVar:
+        case kIROp_GlobalParam:
             return true;
 
         default:
@@ -6350,6 +6357,9 @@ namespace Slang
         case kIROp_GlobalConstant:
             return cloneGlobalConstantImpl(context, builder, cast<IRGlobalConstant>(originalInst), originalValues);
 
+        case kIROp_GlobalParam:
+            return cloneGlobalParamImpl(context, builder, cast<IRGlobalParam>(originalInst), originalValues);
+
         case kIROp_WitnessTable:
             return cloneWitnessTableImpl(context, builder, cast<IRWitnessTable>(originalInst), originalValues);