Remove legacy feature for merging global shader parameters (#1139)

* Remove legacy feature for merging global shader parameters

There is a fair amount of special-case code in the Slang compiler today to deal with the scenario where a programmer declares the "same" shader parameter across two different translation units:

```hlsl
// a.hlsl
Texture2D a;
cbuffer C { float4 c; }
```

```hlsl
// b.hlsl
cbuffer C { float4 c; }
Texture2D b;
```

An important note here is that the declaration of `C` may be in a header file that both `a.hlsl` and `b.hlsl` `#include`, because from the standpoint of the parser and later stages of the compiler, there is no difference between `C` being in an included file vs. it being copy-pasted across both `a.hlsl` and `b.hlsl`.

When a user invokes `slangc a.hlsl b.hlsl` (or the equivalent via the API), then they may decide that it is "obvious" that the shader parameter `C` is the "same" in both `a.hlsl` and `b.hlsl`.
Knowing that the parameter is the "same" may lead them to make certain assumptions:

* They may assume that generated code for entry points in `a.hlsl` and `b.hlsl` will both agree on the exact `register`/`binding` occupied by `C`.

* They may assume that reflection information for their program will only reflect `C` once, and it will reflect it in a way that is applicable to entry points in both `a.hlsl` and `b.hlsl`

* They may assume that the compiler can and should handle this use case even when `C` contains fields with `struct` types that are declared in both `a.hlsl` and `b.hlsl` that have the "same" definition.

* They may assume that in cases where `C` is declared inconsistently between `a.hlsl` and `b.hlsl` the compiler can and will diagnose an error.

Making these assumptions work in practice required a lot of special-case code:

* When composing/linking programs was `ComponentType`s we had to include a special case `LegacyProgram` type that could provide these "do what I mean" semantics, since they are *not* what one would want in the general case for a `CompositeComponentType`.

* During enumeration of global shader parameter in a `LegacyProgram`, we had to detect parameters from distinct modules (translation units) with the same name, and then enforce that they must have the "same" type (via an ad hoc recursive structural type match). No other semantic checking logic needs or uses that kind of structural check.

* During parameter binding generation, we need to handle the case where a single global shader parameter might have multiple declarations, and make sure to collect explicit bindings from all of them (checking for inconsistency) and also to apply generated bindings to all of them.

* The `mapVarToLayout` member in `StructTypeLayout` is a concession to the fact that we might have multiple `VarDecl`s for each field of the struct that represents the global scope, we might need to look up a field and its layout using any of those declarations (much of the need for this field had gone away now that IR passes are largely using IR-based layout).

All of these different special cases added more complex code in many places in the compiler, all to support a scenario that isn't especially common.
Most users won't be affected by the original issue, because they will do one of several things that rule it out:

* Anybody using `slangc` like a stand-in for `fxc` or `dxc` and compiling one translation unit at a time will not suffer from any problems. If/when such users want consistent bindings across translation units, they already use either explicit binding or rely on consistent ordering and implicit binding.

* Anybody who puts all the entry points that get combined into a pass/pipeline in a single file will not have problems. They will automatically get consistent bindings because of Slang's guarantees, and there can't be duplicated declarations when there is only one translation unit.

* Anybody using `import` to factor out common declarations while compiling multiple translation units at once will not be affected. Parameters declared in an `import`ed module are the "same" in a much deeper way that it is trivial for Slang to support.

Only users of the Falcor framework are likely to be affected by this, and they have two easy migration paths: either put related entry points into the same file, or factor common parameters into an `import`ed module.

(It is also worth noting that for command-line `slangc`, it is possible to have a single module with multiple `.slang` files in it, which can all see global declarations like parameters across all the files. Anybody who buys into doing things the Slang Way should have no problem avoiding duplicated declarations)

With the rationale out of the way, the actual change mostly just amounts to deleting lots of code that is no longer needed. An astute reviewer might notice several `assert`-fail conditions where complex Slang features were never actually made to work correctly with this legacy behavior.

A small number of test cases broke with the code changes, but these were tests that specifically exercised the behavior being removed. In the case of the tests around binding/reflection generating, I rewrote the tests to use one of the idomatic workarounds (putting the shared parameters into an `import`ed module), but doing so required me to add support for `#include` when doing pass-through compilation with `fxc`. That logic added a bit more cruft than I had originally hoped to this commit, but having `#include` support when doing pass-through compilation is probably a net win.

* fixup: 64-bit warning

1 files changed, 14 insertions, 499 deletions

diff --git a/source/slang/slang-check-shader.cpp b/source/slang/slang-check-shader.cpp
index c229e8f96..611f6be58 100644
--- a/source/slang/slang-check-shader.cpp
+++ b/source/slang/slang-check-shader.cpp
@@ -569,408 +569,15 @@ namespace Slang
         return entryPoint;
     }
 
-    /// Get the name a variable will use for reflection purposes
-Name* getReflectionName(VarDeclBase* varDecl)
-{
-    if (auto reflectionNameModifier = varDecl->FindModifier<ParameterGroupReflectionName>())
-        return reflectionNameModifier->nameAndLoc.name;
-
-    return varDecl->getName();
-}
-
-// Information tracked when doing a structural
-// match of types.
-struct StructuralTypeMatchStack
-{
-    DeclRef<VarDeclBase>        leftDecl;
-    DeclRef<VarDeclBase>        rightDecl;
-    StructuralTypeMatchStack*   parent;
-};
-
-static void diagnoseParameterTypeMismatch(
-    DiagnosticSink*             sink,
-    StructuralTypeMatchStack*   inStack)
-{
-    SLANG_ASSERT(inStack);
-
-    // The bottom-most entry in the stack should represent
-    // the shader parameters that kicked things off
-    auto stack = inStack;
-    while(stack->parent)
-        stack = stack->parent;
-
-    sink->diagnose(stack->leftDecl, Diagnostics::shaderParameterDeclarationsDontMatch, getReflectionName(stack->leftDecl));
-    sink->diagnose(stack->rightDecl, Diagnostics::seeOtherDeclarationOf, getReflectionName(stack->rightDecl));
-}
-
-// Two types that were expected to match did not.
-// Inform the user with a suitable message.
-static void diagnoseTypeMismatch(
-    DiagnosticSink*             sink,
-    StructuralTypeMatchStack*   inStack)
-{
-    auto stack = inStack;
-    SLANG_ASSERT(stack);
-    diagnoseParameterTypeMismatch(sink, stack);
-
-    auto leftType = GetType(stack->leftDecl);
-    auto rightType = GetType(stack->rightDecl);
-
-    if( stack->parent )
-    {
-        sink->diagnose(stack->leftDecl, Diagnostics::fieldTypeMisMatch, getReflectionName(stack->leftDecl), leftType, rightType);
-        sink->diagnose(stack->rightDecl, Diagnostics::seeOtherDeclarationOf, getReflectionName(stack->rightDecl));
-
-        stack = stack->parent;
-        if( stack )
-        {
-            while( stack->parent )
-            {
-                sink->diagnose(stack->leftDecl, Diagnostics::usedInDeclarationOf, getReflectionName(stack->leftDecl));
-                stack = stack->parent;
-            }
-        }
-    }
-    else
-    {
-        sink->diagnose(stack->leftDecl, Diagnostics::shaderParameterTypeMismatch, leftType, rightType);
-    }
-}
-
-// Two types that were expected to match did not.
-// Inform the user with a suitable message.
-static void diagnoseTypeFieldsMismatch(
-    DiagnosticSink*             sink,
-    DeclRef<Decl> const&        left,
-    DeclRef<Decl> const&        right,
-    StructuralTypeMatchStack*   stack)
-{
-    diagnoseParameterTypeMismatch(sink, stack);
-
-    sink->diagnose(left, Diagnostics::fieldDeclarationsDontMatch, left.GetName());
-    sink->diagnose(right, Diagnostics::seeOtherDeclarationOf, right.GetName());
-
-    if( stack )
-    {
-        while( stack->parent )
-        {
-            sink->diagnose(stack->leftDecl, Diagnostics::usedInDeclarationOf, getReflectionName(stack->leftDecl));
-            stack = stack->parent;
-        }
-    }
-}
-
-static void collectFields(
-    DeclRef<AggTypeDecl>    declRef,
-    List<DeclRef<VarDecl>>& outFields)
-{
-    for( auto fieldDeclRef : getMembersOfType<VarDecl>(declRef) )
-    {
-        if(fieldDeclRef.getDecl()->HasModifier<HLSLStaticModifier>())
-            continue;
-
-        outFields.add(fieldDeclRef);
-    }
-}
-
-static bool validateTypesMatch(
-    DiagnosticSink*             sink,
-    Type*                       left,
-    Type*                       right,
-    StructuralTypeMatchStack*   stack);
-
-static bool validateIntValuesMatch(
-    DiagnosticSink*             sink,
-    IntVal*                     left,
-    IntVal*                     right,
-    StructuralTypeMatchStack*   stack)
-{
-    if(left->EqualsVal(right))
-        return true;
-
-    // TODO: are there other cases we need to handle here?
-
-    diagnoseTypeMismatch(sink, stack);
-    return false;
-}
-
-
-static bool validateValuesMatch(
-    DiagnosticSink*             sink,
-    Val*                        left,
-    Val*                        right,
-    StructuralTypeMatchStack*   stack)
-{
-    if( auto leftType = dynamicCast<Type>(left) )
-    {
-        if( auto rightType = dynamicCast<Type>(right) )
-        {
-            return validateTypesMatch(sink, leftType, rightType, stack);
-        }
-    }
-
-    if( auto leftInt = dynamicCast<IntVal>(left) )
-    {
-        if( auto rightInt = dynamicCast<IntVal>(right) )
-        {
-            return validateIntValuesMatch(sink, leftInt, rightInt, stack);
-        }
-    }
-
-    if( auto leftWitness = dynamicCast<SubtypeWitness>(left) )
-    {
-        if( auto rightWitness = dynamicCast<SubtypeWitness>(right) )
-        {
-            return true;
-        }
-    }
-
-    diagnoseTypeMismatch(sink, stack);
-    return false;
-}
-
-static bool validateGenericSubstitutionsMatch(
-    DiagnosticSink*             sink,
-    GenericSubstitution*        left,
-    GenericSubstitution*        right,
-    StructuralTypeMatchStack*   stack)
-{
-    if( !left )
-    {
-        if( !right )
-        {
-            return true;
-        }
-
-        diagnoseTypeMismatch(sink, stack);
-        return false;
-    }
-
-
-
-    Index argCount = left->args.getCount();
-    if( argCount != right->args.getCount() )
-    {
-        diagnoseTypeMismatch(sink, stack);
-        return false;
-    }
-
-    for( Index aa = 0; aa < argCount; ++aa )
-    {
-        auto leftArg = left->args[aa];
-        auto rightArg = right->args[aa];
-
-        if(!validateValuesMatch(sink, leftArg, rightArg, stack))
-            return false;
-    }
-
-    return true;
-}
-
-static bool validateThisTypeSubstitutionsMatch(
-    DiagnosticSink*             /*sink*/,
-    ThisTypeSubstitution*       /*left*/,
-    ThisTypeSubstitution*       /*right*/,
-    StructuralTypeMatchStack*   /*stack*/)
-{
-    // TODO: actual checking.
-    return true;
-}
-
-static bool validateSpecializationsMatch(
-    DiagnosticSink*             sink,
-    SubstitutionSet             left,
-    SubstitutionSet             right,
-    StructuralTypeMatchStack*   stack)
-{
-    auto ll = left.substitutions;
-    auto rr = right.substitutions;
-    for(;;)
-    {
-        // Skip any global generic substitutions.
-        if(auto leftGlobalGeneric = as<GlobalGenericParamSubstitution>(ll))
-        {
-            ll = leftGlobalGeneric->outer;
-            continue;
-        }
-        if(auto rightGlobalGeneric = as<GlobalGenericParamSubstitution>(rr))
-        {
-            rr = rightGlobalGeneric->outer;
-            continue;
-        }
-
-        // If either ran out, then we expect both to have run out.
-        if(!ll || !rr)
-            return !ll && !rr;
-
-        auto leftSubst = ll;
-        auto rightSubst = rr;
-
-        ll = ll->outer;
-        rr = rr->outer;
-
-        if(auto leftGeneric = as<GenericSubstitution>(leftSubst))
-        {
-            if(auto rightGeneric = as<GenericSubstitution>(rightSubst))
-            {
-                if(validateGenericSubstitutionsMatch(sink, leftGeneric, rightGeneric, stack))
-                {
-                    continue;
-                }
-            }
-        }
-        else if(auto leftThisType = as<ThisTypeSubstitution>(leftSubst))
-        {
-            if(auto rightThisType = as<ThisTypeSubstitution>(rightSubst))
-            {
-                if(validateThisTypeSubstitutionsMatch(sink, leftThisType, rightThisType, stack))
-                {
-                    continue;
-                }
-            }
-        }
-
-        return false;
-    }
-
-    return true;
-}
-
-// Determine if two types "match" for the purposes of `cbuffer` layout rules.
-//
-static bool validateTypesMatch(
-    DiagnosticSink*             sink,
-    Type*                       left,
-    Type*                       right,
-    StructuralTypeMatchStack*   stack)
-{
-    if(left->Equals(right))
-        return true;
-
-    // It is possible that the types don't match exactly, but
-    // they *do* match structurally.
-
-    // Note: the following code will lead to infinite recursion if there
-    // are ever recursive types. We'd need a more refined system to
-    // cache the matches we've already found.
-
-    if( auto leftDeclRefType = as<DeclRefType>(left) )
-    {
-        if( auto rightDeclRefType = as<DeclRefType>(right) )
-        {
-            // Are they references to matching decl refs?
-            auto leftDeclRef = leftDeclRefType->declRef;
-            auto rightDeclRef = rightDeclRefType->declRef;
-
-            // Do the reference the same declaration? Or declarations
-            // with the same name?
-            //
-            // TODO: we should only consider the same-name case if the
-            // declarations come from translation units being compiled
-            // (and not an imported module).
-            if( leftDeclRef.getDecl() == rightDeclRef.getDecl()
-                || leftDeclRef.GetName() == rightDeclRef.GetName() )
-            {
-                // Check that any generic arguments match
-                if( !validateSpecializationsMatch(
-                    sink,
-                    leftDeclRef.substitutions,
-                    rightDeclRef.substitutions,
-                    stack) )
-                {
-                    return false;
-                }
-
-                // Check that any declared fields match too.
-                if( auto leftStructDeclRef = leftDeclRef.as<AggTypeDecl>() )
-                {
-                    if( auto rightStructDeclRef = rightDeclRef.as<AggTypeDecl>() )
-                    {
-                        List<DeclRef<VarDecl>> leftFields;
-                        List<DeclRef<VarDecl>> rightFields;
-
-                        collectFields(leftStructDeclRef, leftFields);
-                        collectFields(rightStructDeclRef, rightFields);
-
-                        Index leftFieldCount = leftFields.getCount();
-                        Index rightFieldCount = rightFields.getCount();
-
-                        if( leftFieldCount != rightFieldCount )
-                        {
-                            diagnoseTypeFieldsMismatch(sink, leftDeclRef, rightDeclRef, stack);
-                            return false;
-                        }
-
-                        for( Index ii = 0; ii < leftFieldCount; ++ii )
-                        {
-                            auto leftField = leftFields[ii];
-                            auto rightField = rightFields[ii];
-
-                            if( leftField.GetName() != rightField.GetName() )
-                            {
-                                diagnoseTypeFieldsMismatch(sink, leftDeclRef, rightDeclRef, stack);
-                                return false;
-                            }
-
-                            auto leftFieldType = GetType(leftField);
-                            auto rightFieldType = GetType(rightField);
-
-                            StructuralTypeMatchStack subStack;
-                            subStack.parent = stack;
-                            subStack.leftDecl = leftField;
-                            subStack.rightDecl = rightField;
-
-                            if(!validateTypesMatch(sink, leftFieldType,rightFieldType, &subStack))
-                                return false;
-                        }
-                    }
-                }
-
-                // Everything seemed to match recursively.
-                return true;
-            }
-        }
-    }
-
-    // If we are looking at `T[N]` and `U[M]` we want to check that
-    // `T` is structurally equivalent to `U` and `N` is the same as `M`.
-    else if( auto leftArrayType = as<ArrayExpressionType>(left) )
+        /// Get the name a variable will use for reflection purposes
+    Name* getReflectionName(VarDeclBase* varDecl)
     {
-        if( auto rightArrayType = as<ArrayExpressionType>(right) )
-        {
-            if(!validateTypesMatch(sink, leftArrayType->baseType, rightArrayType->baseType, stack) )
-                return false;
-
-            if(!validateValuesMatch(sink, leftArrayType->ArrayLength, rightArrayType->ArrayLength, stack))
-                return false;
+        if (auto reflectionNameModifier = varDecl->FindModifier<ParameterGroupReflectionName>())
+            return reflectionNameModifier->nameAndLoc.name;
 
-            return true;
-        }
+        return varDecl->getName();
     }
 
-    diagnoseTypeMismatch(sink, stack);
-    return false;
-}
-
-// This function is supposed to determine if two global shader
-// parameter declarations represent the same logical parameter
-// (so that they should get the exact same binding(s) allocated).
-//
-static bool doesParameterMatch(
-    DiagnosticSink*         sink,
-    DeclRef<VarDeclBase>    varDeclRef,
-    DeclRef<VarDeclBase>    existingVarDeclRef)
-{
-    StructuralTypeMatchStack stack;
-    stack.parent = nullptr;
-    stack.leftDecl = varDeclRef;
-    stack.rightDecl = existingVarDeclRef;
-
-    validateTypesMatch(sink, GetType(varDeclRef), GetType(existingVarDeclRef), &stack);
-
-    return true;
-}
-
     void Module::_collectShaderParams()
     {
         auto moduleDecl = m_moduleDecl;
@@ -1003,7 +610,7 @@ static bool doesParameterMatch(
 
                 // At this point we know we have a global shader parameter.
 
-                GlobalShaderParamInfo shaderParamInfo;
+                ShaderParamInfo shaderParamInfo;
                 shaderParamInfo.paramDeclRef = makeDeclRef(globalVar.Ptr());
 
                 // We need to consider what specialization parameters
@@ -1072,102 +679,6 @@ static bool doesParameterMatch(
     }
 
 
-        /// Enumerate the parameters of a `LegacyProgram`.
-    void LegacyProgram::_collectShaderParams(DiagnosticSink* sink)
-    {
-        // We need to collect all of the global shader parameters
-        // referenced by the compile request, and for each we
-        // need to do a few things:
-        //
-        // * We need to determine if the parameter is a duplicate/redeclaration
-        //   of the "same" parameter in another translation unit, and collapse
-        //   those into one logical shader parameter if so.
-        //
-        // * We need to determine what existential type slots are introduced
-        //   by the parameter, and associate that information with the parameter.
-        //
-        // To deal with the first issue, we will maintain a map from a parameter
-        // name to the index of an existing parameter with that name.
-        //
-        // TODO: Eventually we should deprecate support for the
-        // deduplication feature of `LegaqcyProgram`, at which point
-        // this entire type and all its complications can be eliminated
-        // from the code (that includes a lot of support in the "parameter
-        // binding" step for shader parameters with multiple declarations).
-        // Until that point this type will have a fair amount of duplication
-        // with stuff in `Module` and `CompositeComponentType`.
-
-        // We use a dictionary to keep track of any shader parameter
-        // we've alrady collected with a given name.
-        //
-        Dictionary<Name*, Int> mapNameToParamIndex;
-
-        for( auto translationUnit : m_translationUnits )
-        {
-            auto module = translationUnit->getModule();
-            auto moduleDecl = module->getModuleDecl();
-            for( auto globalVar : moduleDecl->getMembersOfType<VarDecl>() )
-            {
-                // We do not want to consider global variable declarations
-                // that don't represents shader parameters. This includes
-                // things like `static` globals and `groupshared` variables.
-                //
-                if(!isGlobalShaderParameter(globalVar))
-                    continue;
-
-                // This declaration may represent the same logical parameter
-                // as a declaration that came from a different translation unit.
-                // If that is the case, we want to re-use the same `ShaderParamInfo`
-                // across both parameters.
-                //
-                // TODO: This logic currently detects *any* global-scope parameters
-                // with matching names, but it should eventually be narrowly
-                // scoped so that it only applies to parameters from unnamed modules
-                // (that is, modules that represent directly-compiled shader files
-                // and not `import`ed code).
-                //
-                // First we look for an existing entry matching the name
-                // of this parameter:
-                //
-                auto paramName = getReflectionName(globalVar);
-                Int existingParamIndex = -1;
-                if( mapNameToParamIndex.TryGetValue(paramName, existingParamIndex) )
-                {
-                    // If the parameters have the same name, but don't "match" according to some reasonable rules,
-                    // then we will treat them as distinct global parameters.
-                    //
-                    // Note: all of the mismatch cases currently report errors, so that
-                    // compilation will fail on a mismatch.
-                    //
-                    auto& existingParam = m_shaderParams[existingParamIndex];
-                    if( doesParameterMatch(sink, makeDeclRef(globalVar.Ptr()), existingParam.paramDeclRef) )
-                    {
-                        // If we hit this case, then we had a match, and we should
-                        // consider the new variable to be a redclaration of
-                        // the existing one.
-
-                        existingParam.additionalParamDeclRefs.add(
-                            makeDeclRef(globalVar.Ptr()));
-                        continue;
-                    }
-                }
-
-                Int newParamIndex = Int(m_shaderParams.getCount());
-                mapNameToParamIndex.Add(paramName, newParamIndex);
-
-                GlobalShaderParamInfo shaderParamInfo;
-                shaderParamInfo.paramDeclRef = makeDeclRef(globalVar.Ptr());
-
-                _collectExistentialSpecializationParamsForShaderParam(
-                    shaderParamInfo,
-                    m_specializationParams,
-                    makeDeclRef(globalVar.Ptr()));
-
-                m_shaderParams.add(shaderParamInfo);
-            }
-        }
-    }
-
         /// Create a new component type based on `inComponentType`, but with all its requiremetns filled.
     RefPtr<ComponentType> fillRequirements(
         ComponentType* inComponentType)
@@ -1234,7 +745,6 @@ static bool doesParameterMatch(
         // compiler behavior (at least for now).
         //
         auto linkage = compileRequest->getLinkage();
-        auto sink = compileRequest->getSink();
 
         RefPtr<ComponentType> globalComponentType;
         if(compileRequest->translationUnits.getCount() == 1)
@@ -1248,10 +758,15 @@ static bool doesParameterMatch(
         }
         else
         {
-            globalComponentType = new LegacyProgram(
+            List<RefPtr<ComponentType>> translationUnitComponentTypes;
+            for( auto tu : compileRequest->translationUnits )
+            {
+                translationUnitComponentTypes.add(tu->getModule());
+            }
+
+            globalComponentType = CompositeComponentType::create(
                 linkage,
-                compileRequest->translationUnits,
-                sink);
+                translationUnitComponentTypes);
         }
 
         return fillRequirements(globalComponentType);