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, 255 insertions, 37 deletions

diff --git a/source/slang/ir-legalize-types.cpp b/source/slang/ir-legalize-types.cpp
index 901d8705b..a97cc0393 100644
--- a/source/slang/ir-legalize-types.cpp
+++ b/source/slang/ir-legalize-types.cpp
@@ -335,31 +335,31 @@ static LegalVal legalizeStore(
     }
 }
 
-static LegalVal legalizeFieldAddress(
-    IRTypeLegalizationContext*    context,
+static LegalVal legalizeFieldExtract(
+    IRTypeLegalizationContext*  context,
     LegalType                   type,
-    LegalVal                    legalPtrOperand,
+    LegalVal                    legalStructOperand,
     IRStructKey*                fieldKey)
 {
     auto builder = context->builder;
 
-    switch (legalPtrOperand.flavor)
+    switch (legalStructOperand.flavor)
     {
     case LegalVal::Flavor::none:
         return LegalVal();
 
     case LegalVal::Flavor::simple:
         return LegalVal::simple(
-            builder->emitFieldAddress(
+            builder->emitFieldExtract(
                 type.getSimple(),
-                legalPtrOperand.getSimple(),
+                legalStructOperand.getSimple(),
                 fieldKey));
 
     case LegalVal::Flavor::pair:
         {
             // There are two sides, the ordinary and the special,
             // and we basically just dispatch to both of them.
-            auto pairVal = legalPtrOperand.getPair();
+            auto pairVal = legalStructOperand.getPair();
             auto pairInfo = pairVal->pairInfo;
             auto pairElement = pairInfo->findElement(fieldKey);
             if (!pairElement)
@@ -387,7 +387,7 @@ static LegalVal legalizeFieldAddress(
 
             if (pairElement->flags & PairInfo::kFlag_hasOrdinary)
             {
-                ordinaryVal = legalizeFieldAddress(
+                ordinaryVal = legalizeFieldExtract(
                     context,
                     ordinaryType,
                     pairVal->ordinaryVal,
@@ -396,7 +396,7 @@ static LegalVal legalizeFieldAddress(
 
             if (pairElement->flags & PairInfo::kFlag_hasSpecial)
             {
-                specialVal = legalizeFieldAddress(
+                specialVal = legalizeFieldExtract(
                     context,
                     specialType,
                     pairVal->specialVal,
@@ -413,7 +413,7 @@ static LegalVal legalizeFieldAddress(
             // corresponding to a field. We will handle
             // this by simply returning the corresponding
             // element from the operand.
-            auto ptrTupleInfo = legalPtrOperand.getTuple();
+            auto ptrTupleInfo = legalStructOperand.getTuple();
             for (auto ee : ptrTupleInfo->elements)
             {
                 if (ee.key == fieldKey)
@@ -435,7 +435,7 @@ static LegalVal legalizeFieldAddress(
     }
 }
 
-static LegalVal legalizeFieldAddress(
+static LegalVal legalizeFieldExtract(
     IRTypeLegalizationContext*    context,
     LegalType                   type,
     LegalVal                    legalPtrOperand,
@@ -445,38 +445,38 @@ static LegalVal legalizeFieldAddress(
     // the "field" argument.
     auto fieldKey = legalFieldOperand.getSimple();
 
-    return legalizeFieldAddress(
+    return legalizeFieldExtract(
         context,
         type,
         legalPtrOperand,
         (IRStructKey*) fieldKey);
 }
 
-static LegalVal legalizeFieldExtract(
-    IRTypeLegalizationContext*  context,
+static LegalVal legalizeFieldAddress(
+    IRTypeLegalizationContext*    context,
     LegalType                   type,
-    LegalVal                    legalStructOperand,
+    LegalVal                    legalPtrOperand,
     IRStructKey*                fieldKey)
 {
     auto builder = context->builder;
 
-    switch (legalStructOperand.flavor)
+    switch (legalPtrOperand.flavor)
     {
     case LegalVal::Flavor::none:
         return LegalVal();
 
     case LegalVal::Flavor::simple:
         return LegalVal::simple(
-            builder->emitFieldExtract(
+            builder->emitFieldAddress(
                 type.getSimple(),
-                legalStructOperand.getSimple(),
+                legalPtrOperand.getSimple(),
                 fieldKey));
 
     case LegalVal::Flavor::pair:
         {
             // There are two sides, the ordinary and the special,
             // and we basically just dispatch to both of them.
-            auto pairVal = legalStructOperand.getPair();
+            auto pairVal = legalPtrOperand.getPair();
             auto pairInfo = pairVal->pairInfo;
             auto pairElement = pairInfo->findElement(fieldKey);
             if (!pairElement)
@@ -504,7 +504,7 @@ static LegalVal legalizeFieldExtract(
 
             if (pairElement->flags & PairInfo::kFlag_hasOrdinary)
             {
-                ordinaryVal = legalizeFieldExtract(
+                ordinaryVal = legalizeFieldAddress(
                     context,
                     ordinaryType,
                     pairVal->ordinaryVal,
@@ -513,7 +513,7 @@ static LegalVal legalizeFieldExtract(
 
             if (pairElement->flags & PairInfo::kFlag_hasSpecial)
             {
-                specialVal = legalizeFieldExtract(
+                specialVal = legalizeFieldAddress(
                     context,
                     specialType,
                     pairVal->specialVal,
@@ -530,7 +530,7 @@ static LegalVal legalizeFieldExtract(
             // corresponding to a field. We will handle
             // this by simply returning the corresponding
             // element from the operand.
-            auto ptrTupleInfo = legalStructOperand.getTuple();
+            auto ptrTupleInfo = legalPtrOperand.getTuple();
             for (auto ee : ptrTupleInfo->elements)
             {
                 if (ee.key == fieldKey)
@@ -546,13 +546,27 @@ static LegalVal legalizeFieldExtract(
             UNREACHABLE_RETURN(LegalVal());
         }
 
+    case LegalVal::Flavor::implicitDeref:
+        {
+            // The original value had a level of indirection
+            // that is now being removed, so should not be
+            // able to get at the *address* of the field any
+            // more, and need to resign ourselves to just
+            // getting at the field *value* and then
+            // adding an implicit dereference on top of that.
+            //
+            auto implicitDerefVal = legalPtrOperand.getImplicitDeref();
+
+            return LegalVal::implicitDeref(legalizeFieldExtract(context,type, implicitDerefVal, fieldKey));
+        }
+
     default:
         SLANG_UNEXPECTED("unhandled");
         UNREACHABLE_RETURN(LegalVal());
     }
 }
 
-static LegalVal legalizeFieldExtract(
+static LegalVal legalizeFieldAddress(
     IRTypeLegalizationContext*    context,
     LegalType                   type,
     LegalVal                    legalPtrOperand,
@@ -562,13 +576,125 @@ static LegalVal legalizeFieldExtract(
     // the "field" argument.
     auto fieldKey = legalFieldOperand.getSimple();
 
-    return legalizeFieldExtract(
+    return legalizeFieldAddress(
         context,
         type,
         legalPtrOperand,
         (IRStructKey*) fieldKey);
 }
 
+static LegalVal legalizeGetElement(
+    IRTypeLegalizationContext*  context,
+    LegalType                   type,
+    LegalVal                    legalPtrOperand,
+    IRInst*                    indexOperand)
+{
+    auto builder = context->builder;
+
+    switch (legalPtrOperand.flavor)
+    {
+    case LegalVal::Flavor::none:
+        return LegalVal();
+
+    case LegalVal::Flavor::simple:
+        return LegalVal::simple(
+            builder->emitElementExtract(
+                type.getSimple(),
+                legalPtrOperand.getSimple(),
+                indexOperand));
+
+    case LegalVal::Flavor::pair:
+        {
+            // There are two sides, the ordinary and the special,
+            // and we basically just dispatch to both of them.
+            auto pairVal = legalPtrOperand.getPair();
+            auto pairInfo = pairVal->pairInfo;
+
+            LegalType ordinaryType = type;
+            LegalType specialType = type;
+            if (type.flavor == LegalType::Flavor::pair)
+            {
+                auto pairType = type.getPair();
+                ordinaryType = pairType->ordinaryType;
+                specialType = pairType->specialType;
+            }
+
+            LegalVal ordinaryVal = legalizeGetElement(
+                context,
+                ordinaryType,
+                pairVal->ordinaryVal,
+                indexOperand);
+
+            LegalVal specialVal = legalizeGetElement(
+                context,
+                specialType,
+                pairVal->specialVal,
+                indexOperand);
+
+            return LegalVal::pair(ordinaryVal, specialVal, pairInfo);
+        }
+        break;
+
+    case LegalVal::Flavor::tuple:
+        {
+            // The operand is a tuple of pointer-like
+            // values, we want to extract the element
+            // corresponding to a field. We will handle
+            // this by simply returning the corresponding
+            // element from the operand.
+            auto ptrTupleInfo = legalPtrOperand.getTuple();
+
+            RefPtr<TuplePseudoVal> resTupleInfo = new TuplePseudoVal();
+
+            auto tupleType = type.getTuple();
+            SLANG_ASSERT(tupleType);
+
+            auto elemCount = ptrTupleInfo->elements.Count();
+            SLANG_ASSERT(elemCount == tupleType->elements.Count());
+
+            for(UInt ee = 0; ee < elemCount; ++ee)
+            {
+                auto ptrElem = ptrTupleInfo->elements[ee];
+                auto elemType = tupleType->elements[ee].type;
+
+                TuplePseudoVal::Element resElem;
+                resElem.key = ptrElem.key;
+                resElem.val = legalizeGetElement(
+                    context,
+                    elemType,
+                    ptrElem.val,
+                    indexOperand);
+
+                resTupleInfo->elements.Add(resElem);
+            }
+
+            return LegalVal::tuple(resTupleInfo);
+        }
+
+    default:
+        SLANG_UNEXPECTED("unhandled");
+        UNREACHABLE_RETURN(LegalVal());
+    }
+}
+
+static LegalVal legalizeGetElement(
+    IRTypeLegalizationContext*  context,
+    LegalType                   type,
+    LegalVal                    legalPtrOperand,
+    LegalVal                    legalIndexOperand)
+{
+    // We don't expect any legalization to affect
+    // the "index" argument.
+    auto indexOperand = legalIndexOperand.getSimple();
+
+    return legalizeGetElement(
+        context,
+        type,
+        legalPtrOperand,
+        indexOperand);
+}
+
+
 static LegalVal legalizeGetElementPtr(
     IRTypeLegalizationContext*  context,
     LegalType                   type,
@@ -657,6 +783,23 @@ static LegalVal legalizeGetElementPtr(
             return LegalVal::tuple(resTupleInfo);
         }
 
+    case LegalVal::Flavor::implicitDeref:
+        {
+            // The original value used to be a pointer to an array,
+            // and somebody is trying to get at an element pointer.
+            // Now we just have an array (wrapped with an implicit
+            // dereference) and need to just fetch the chosen element
+            // instead (and then wrapp the element value with an
+            // implicit dereference).
+            //
+            auto implicitDerefVal = legalPtrOperand.getImplicitDeref();
+            return LegalVal::implicitDeref(legalizeGetElement(
+                context,
+                type,
+                implicitDerefVal,
+                indexOperand));
+        }
+
     default:
         SLANG_UNEXPECTED("unhandled");
         UNREACHABLE_RETURN(LegalVal());
@@ -816,6 +959,9 @@ static LegalVal legalizeInst(
     case kIROp_FieldExtract:
         return legalizeFieldExtract(context, type, args[0], args[1]);
 
+    case kIROp_getElement:
+        return legalizeGetElement(context, type, args[0], args[1]);
+
     case kIROp_getElementPtr:
         return legalizeGetElementPtr(context, type, args[0], args[1]);
 
@@ -965,6 +1111,9 @@ static LegalVal legalizeGlobalConstant(
     IRTypeLegalizationContext*  context,
     IRGlobalConstant*           irGlobalConstant);
 
+static LegalVal legalizeGlobalParam(
+    IRTypeLegalizationContext*  context,
+    IRGlobalParam*              irGlobalParam);
 
 static LegalVal legalizeInst(
     IRTypeLegalizationContext*  context,
@@ -992,6 +1141,9 @@ static LegalVal legalizeInst(
     case kIROp_GlobalConstant:
         return legalizeGlobalConstant(context, cast<IRGlobalConstant>(inst));
 
+    case kIROp_GlobalParam:
+        return legalizeGlobalParam(context, cast<IRGlobalParam>(inst));
+
     default:
         break;
     }
@@ -1184,6 +1336,28 @@ static LegalVal declareSimpleVar(
         }
         break;
 
+    case kIROp_GlobalConstant:
+        {
+            auto globalConst = builder->createGlobalConstant(type);
+            globalConst->removeFromParent();
+            globalConst->insertBefore(context->insertBeforeGlobal);
+
+            irVar = globalConst;
+            legalVarVal = LegalVal::simple(globalConst);
+        }
+        break;
+
+    case kIROp_GlobalParam:
+        {
+            auto globalParam = builder->createGlobalParam(type);
+            globalParam->removeFromParent();
+            globalParam->insertBefore(context->insertBeforeGlobal);
+
+            irVar = globalParam;
+            legalVarVal = LegalVal::simple(globalParam);
+        }
+        break;
+
     case kIROp_Var:
         {
             auto localVar = builder->emitVar(type);
@@ -1355,9 +1529,6 @@ static LegalVal legalizeGlobalVar(
         context,
         irGlobalVar->getDataType()->getValueType());
 
-    RefPtr<VarLayout> varLayout = findVarLayout(irGlobalVar);
-    RefPtr<TypeLayout> typeLayout = varLayout ? varLayout->typeLayout : nullptr;
-
     switch (legalValueType.flavor)
     {
     case LegalType::Flavor::simple:
@@ -1373,21 +1544,12 @@ static LegalVal legalizeGlobalVar(
         {
             context->insertBeforeGlobal = irGlobalVar->getNextInst();
 
-            LegalVarChain* varChain = nullptr;
-            LegalVarChain varChainStorage;
-            if (varLayout)
-            {
-                varChainStorage.next = nullptr;
-                varChainStorage.varLayout = varLayout;
-                varChain = &varChainStorage;
-            }
-
             IRGlobalNameInfo globalNameInfo;
             globalNameInfo.globalVar = irGlobalVar;
             globalNameInfo.counter = 0;
 
             UnownedStringSlice nameHint = findNameHint(irGlobalVar);
-            LegalVal newVal = declareVars(context, kIROp_GlobalVar, legalValueType, typeLayout, varChain, nameHint, &globalNameInfo);
+            LegalVal newVal = declareVars(context, kIROp_GlobalVar, legalValueType, nullptr, nullptr, nameHint, &globalNameInfo);
 
             // Register the new value as the replacement for the old
             registerLegalizedValue(context, irGlobalVar, newVal);
@@ -1445,6 +1607,62 @@ static LegalVal legalizeGlobalConstant(
     }
 }
 
+static LegalVal legalizeGlobalParam(
+    IRTypeLegalizationContext*  context,
+    IRGlobalParam*              irGlobalParam)
+{
+    // Legalize the type for the variable's value
+    auto legalValueType = legalizeType(
+        context,
+        irGlobalParam->getFullType());
+
+    RefPtr<VarLayout> varLayout = findVarLayout(irGlobalParam);
+    RefPtr<TypeLayout> typeLayout = varLayout ? varLayout->typeLayout : nullptr;
+
+    switch (legalValueType.flavor)
+    {
+    case LegalType::Flavor::simple:
+        // Easy case: the type is usable as-is, and we
+        // should just do that.
+        irGlobalParam->setFullType(legalValueType.getSimple());
+        return LegalVal::simple(irGlobalParam);
+
+    default:
+        {
+            context->insertBeforeGlobal = irGlobalParam->getNextInst();
+
+            LegalVarChain* varChain = nullptr;
+            LegalVarChain varChainStorage;
+            if (varLayout)
+            {
+                varChainStorage.next = nullptr;
+                varChainStorage.varLayout = varLayout;
+                varChain = &varChainStorage;
+            }
+
+            IRGlobalNameInfo globalNameInfo;
+            globalNameInfo.globalVar = irGlobalParam;
+            globalNameInfo.counter = 0;
+
+            // TODO: need to handle initializer here!
+
+            UnownedStringSlice nameHint = findNameHint(irGlobalParam);
+            LegalVal newVal = declareVars(context, kIROp_GlobalParam, legalValueType, typeLayout, varChain, nameHint, &globalNameInfo);
+
+            // Register the new value as the replacement for the old
+            registerLegalizedValue(context, irGlobalParam, newVal);
+
+            // Remove the old global from the module.
+            irGlobalParam->removeFromParent();
+            context->replacedInstructions.Add(irGlobalParam);
+
+            return newVal;
+        }
+        break;
+    }
+}
+
+
 static void legalizeTypes(
     IRTypeLegalizationContext*    context)
 {