Cleanups to `ParameterBlock<T>` behavior. (#304)

* Cleanups to `ParameterBlock<T>` behavior.

These add some more realistic tests using the `ParameterBlock<T>` support, and show that it can work with the "rewriter" mode.

Unfortunately, this code does *not* currently work with the rewriter + the IR at once. That will need to be fixed in a follow-on change, because I now see that the root problem is pretty ugly.

* cleanup

9 files changed, 362 insertions, 67 deletions

diff --git a/source/slang/ast-legalize.cpp b/source/slang/ast-legalize.cpp
index eaceecc00..0c9d6154e 100644
--- a/source/slang/ast-legalize.cpp
+++ b/source/slang/ast-legalize.cpp
@@ -797,24 +797,9 @@ struct LoweringVisitor
             lowerTypeEx(type->valueType));
     }
 
-    RefPtr<Type> visitParameterBlockType(ParameterBlockType* type)
-    {
-        // TODO: When doing AST-to-AST lowering, we want to lower
-        // a `ParameterBlock<T>` just like a `ConstantBuffer<T>`.
-        //
-        // HACK: for now we will try to simply lower the type
-        // directly to its stated element type, and see how
-        // that works.
-
-        return lowerTypeEx(type->getElementType());
-//        return getSession()->getConstantBufferType(
-//            lowerType(type->getElementType());
-    }
-
     RefPtr<Type> transformSyntaxField(Type* type)
     {
-        // TODO: how to handle this...
-        return type;
+        return lowerAndLegalizeSimpleType(type);
     }
 
     RefPtr<Val> visitIRProxyVal(IRProxyVal* val)
@@ -1807,15 +1792,73 @@ struct LoweringVisitor
 
     static LegalExpr maybeReifyTuple(
         LegalExpr       legalExpr,
-        LegalType       expectedType)
+        LegalType       expectedLegalType)
     {
-        if (expectedType.flavor != LegalType::Flavor::simple)
+        if (expectedLegalType.flavor != LegalType::Flavor::simple)
             return legalExpr;
 
+        RefPtr<Type> expectedType = expectedLegalType.getSimple();
+        if(auto errorType = expectedType->As<ErrorType>())
+        {
+            return legalExpr;
+        }
+
         if (legalExpr.getFlavor() == LegalExpr::Flavor::simple)
             return legalExpr;
 
-        return LegalExpr(reifyTuple(legalExpr, expectedType.getSimple()));
+        return LegalExpr(reifyTuple(legalExpr, expectedLegalType.getSimple()));
+    }
+
+    // This function exists to work around cases where `addArgs` gets called
+    // and the structure of the type expected in context (the legalized parameter
+    // type) differs from the structure of the actual argument.
+    //
+    // This function ignores type information and just adds things based on
+    // what is present in the actual expression.
+    void addArgsWorkaround(
+        ExprWithArgsBase*   callExpr,
+        LegalExpr           argExpr)
+    {
+
+        switch (argExpr.getFlavor())
+        {
+        case LegalExpr::Flavor::none:
+            break;
+
+        case LegalExpr::Flavor::simple:
+            addArg(callExpr, argExpr.getSimple());
+            break;
+
+        case LegalExpr::Flavor::tuple:
+            {
+                auto aa = argExpr.getTuple();
+                auto elementCount = aa->elements.Count();
+                for (UInt ee = 0; ee < elementCount; ++ee)
+                {
+                    addArgsWorkaround(callExpr, aa->elements[ee].expr);
+                }
+            }
+            break;
+
+        case LegalExpr::Flavor::pair:
+            {
+                auto aa = argExpr.getPair();
+                addArgsWorkaround(callExpr, aa->ordinary);
+                addArgsWorkaround(callExpr, aa->special);
+            }
+            break;
+
+        case LegalExpr::Flavor::implicitDeref:
+            {
+                auto aa = argExpr.getImplicitDeref();
+                addArgsWorkaround(callExpr, aa->valueExpr);
+            }
+            break;
+
+        default:
+            SLANG_UNEXPECTED("unhandled case");
+            break;
+        }
     }
 
     void addArgs(
@@ -1827,7 +1870,10 @@ struct LoweringVisitor
 
         if (argExpr.getFlavor() != argType.flavor)
         {
-            SLANG_UNEXPECTED("expression and type do not match");
+            // A mismatch may also arise if we are in the `-no-checking` mode,
+            // so that we are making a call that didn't type-check.
+            addArgsWorkaround(callExpr, argExpr);
+            return;
         }
 
         switch (argExpr.getFlavor())
@@ -1900,6 +1946,29 @@ struct LoweringVisitor
         return LegalExpr(lowerCallExpr(loweredExpr, expr));
     }
 
+    LegalExpr visitHiddenImplicitCastExpr(
+        HiddenImplicitCastExpr* expr)
+    {
+        LegalExpr legalArg = legalizeExpr(expr->Arguments[0]);
+        if(legalArg.getFlavor() == LegalExpr::Flavor::simple)
+        {
+            InvokeExpr* loweredExpr = (InvokeExpr*) expr->getClass().createInstance();
+            lowerExprCommon(loweredExpr, expr);
+            loweredExpr->FunctionExpr = legalizeSimpleExpr(expr->FunctionExpr);
+            addArg(loweredExpr, legalArg.getSimple());
+            return LegalExpr(loweredExpr);
+        }
+        else
+        {
+            // If we hit this case, then there seems to have been a type-checking
+            // error around a type that needed to be desugared. We want to use
+            // the original expression rather than hide it behind a cast, because
+            // it might need to be unpacked into multiple arguments for a call, etc.
+            //
+            return legalArg;
+        }
+    }
+
     LegalExpr visitSelectExpr(
         SelectExpr* expr)
     {
@@ -2476,6 +2545,7 @@ struct LoweringVisitor
         RefPtr<Type>    type)
     {
         auto typeType = new TypeType();
+        typeType->setSession(getSession());
         typeType->type = type;
 
         auto result = new SharedTypeExpr();
@@ -3320,7 +3390,6 @@ struct LoweringVisitor
                     typeLayout,
                     legalInit,
                     legalTypeExpr);
-
             }
             break;
 
@@ -3329,8 +3398,17 @@ struct LoweringVisitor
                 auto implicitDerefType = legalType.getImplicitDeref();
 
                 auto valueType = implicitDerefType->valueType;
-                auto valueTypeLayout = getDerefTypeLayout(typeLayout);
-                SLANG_ASSERT(valueTypeLayout || !typeLayout);
+
+                // Don't apply dereferencing to the type layout, because
+                // other steps will also implicitly remove wrappers (like
+                // parameter groups) and this could mess up the final
+                // type layout for a variable.
+                //
+                // Instead, any other "unwrapping" that needs to occur
+                // when declaring variables should be handled in the
+                // case for the specific type (e.g., when extracting
+                // fields for a tuple, we should auto-dereference).
+                auto valueTypeLayout = typeLayout;
                 auto valueInit = deref(legalInit);
 
                 LegalExpr valueExpr = declareVars(
diff --git a/source/slang/check.cpp b/source/slang/check.cpp
index c1893423a..4840ae30d 100644
--- a/source/slang/check.cpp
+++ b/source/slang/check.cpp
@@ -1005,6 +1005,40 @@ namespace Slang
                 }
 
             }
+
+            // Are we converting from a parameter group type to its element type?
+            if(auto fromParameterGroupType = fromType->As<ParameterGroupType>())
+            {
+                auto fromElementType = fromParameterGroupType->getElementType();
+
+                // If we have, e.g., `ConstantBuffer<A>` and we want to convert
+                // to `B`, where conversion from `A` to `B` is possible, then
+                // we will do so here.
+
+                ConversionCost subCost = 0;
+                if(CanCoerce(toType, fromElementType, &subCost))
+                {
+                    if(outCost)
+                        *outCost = subCost + kConversionCost_ImplicitDereference;
+
+                    if(outToExpr)
+                    {
+                        auto derefExpr = new DerefExpr();
+                        derefExpr->base = fromExpr;
+                        derefExpr->type = QualType(fromElementType);
+
+                        return TryCoerceImpl(
+                            toType,
+                            outToExpr,
+                            fromElementType,
+                            derefExpr,
+                            nullptr);
+                    }
+                    return true;
+                }
+            }
+
+
             // Look for an initializer/constructor declaration in the target type,
             // which is marked as usable for implicit conversion, and which takes
             // the source type as an argument.
@@ -1171,6 +1205,7 @@ namespace Slang
             RefPtr<TypeCastExpr> castExpr = createImplicitCastExpr();
 
             auto typeType = new TypeType();
+            typeType->setSession(getSession());
             typeType->type = toType;
 
             auto typeExpr = new SharedTypeExpr();
diff --git a/source/slang/emit.cpp b/source/slang/emit.cpp
index b9aa3c027..09d209f6e 100644
--- a/source/slang/emit.cpp
+++ b/source/slang/emit.cpp
@@ -96,7 +96,9 @@ struct SharedEmitContext
 
     ExtensionUsageTracker extensionUsageTracker;
 
+    UInt uniqueIDCounter = 1;
     Dictionary<IRValue*, UInt> mapIRValueToID;
+    Dictionary<Decl*, UInt> mapDeclToID;
 
     HashSet<Decl*> irDeclsVisited;
 
@@ -503,6 +505,23 @@ struct EmitVisitor
         emitName(name, SourceLoc());
     }
 
+    void emitName(
+        Decl*               decl,
+        SourceLoc const&    loc)
+    {
+        if(auto name = decl->getName())
+            emitName(name, loc);
+
+        Emit("_S");
+        Emit(getID(decl));
+    }
+
+    void emitName(
+        Decl*               decl)
+    {
+        emitName(decl, SourceLoc());
+    }
+
     void Emit(IntegerLiteralValue value)
     {
         char buffer[32];
@@ -3571,31 +3590,95 @@ struct EmitVisitor
 
             auto offsetResource = rr;
 
-            if(kind != LayoutResourceKind::Uniform)
+            if(layout
+                && kind != LayoutResourceKind::Uniform)
             {
                 // Add the base index from the cbuffer into the index of the field
                 //
                 // TODO(tfoley): consider maybe not doing this, since it actually
                 // complicates logic around constant buffers...
 
-                // If the member of the cbuffer uses a resource, it had better
-                // appear as part of the cubffer layout as well.
+                // If the member of the cbuffer uses a resource, we would typically
+                // expect to see that the `cbuffer` itself shows up as using that
+                // resource too.
                 auto cbufferResource = layout->FindResourceInfo(kind);
-                SLANG_RELEASE_ASSERT(cbufferResource);
-
-                offsetResource.index += cbufferResource->index;
-                offsetResource.space += cbufferResource->space;
+                if(cbufferResource)
+                {
+                    offsetResource.index += cbufferResource->index;
+                    offsetResource.space += cbufferResource->space;
+                }
             }
 
             emitHLSLRegisterSemantic(offsetResource, "packoffset");
         }
     }
 
+    void emitHLSLParameterBlockDecl(
+        RefPtr<VarDeclBase>             varDecl,
+        RefPtr<ParameterBlockType>      parameterBlockType,
+        RefPtr<VarLayout>               layout)
+    {
+        Emit("cbuffer ");
+
+        emitName(varDecl);
+
+        // We expect to always have layout information
+        layout = maybeFetchLayout(varDecl, layout);
+        SLANG_RELEASE_ASSERT(layout);
+
+        // We expect the layout to be for a parameter group type...
+        RefPtr<ParameterGroupTypeLayout> bufferLayout = layout->typeLayout.As<ParameterGroupTypeLayout>();
+        SLANG_RELEASE_ASSERT(bufferLayout);
+
+        EmitSemantics(varDecl, kESemanticMask_None);
+
+        auto info = layout->FindResourceInfo(LayoutResourceKind::ConstantBuffer);
+        SLANG_RELEASE_ASSERT(info);
+        emitHLSLRegisterSemantic(*info);
+
+        Emit("\n{\n");
+
+        // The user wrote this declaration as, e.g.:
+        //
+        //      ParameterBlock<Foo> gFoo;
+        //
+        // and we are desugaring it into something like:
+        //
+        //      cbuffer anon0 { Foo gFoo; }
+        //
+
+        RefPtr<Type> elementType = parameterBlockType->elementType;
+        RefPtr<TypeLayout> elementTypeLayout = bufferLayout->elementTypeLayout;
+
+        EmitType(elementType, varDecl->getName());
+
+        // The layout for the field ends up coming from the layout
+        // for the parameter block as a whole.
+        emitHLSLParameterGroupFieldLayoutSemantics(nullptr, layout);
+
+        Emit(";\n");
+        Emit("}\n");
+    }
+
     void emitHLSLParameterGroupDecl(
         RefPtr<VarDeclBase>             varDecl,
         RefPtr<ParameterGroupType>      parameterGroupType,
         RefPtr<VarLayout>               layout)
     {
+        if( auto parameterBlockType = parameterGroupType->As<ParameterBlockType>())
+        {
+            emitHLSLParameterBlockDecl(varDecl, parameterBlockType, layout);
+            return;
+        }
+        if( auto textureBufferType = parameterGroupType->As<TextureBufferType>() )
+        {
+            Emit("tbuffer ");
+        }
+        else
+        {
+            Emit("cbuffer ");
+        }
+
         // The data type that describes where stuff in the constant buffer should go
         RefPtr<Type> dataType = parameterGroupType->elementType;
 
@@ -3610,20 +3693,16 @@ struct EmitVisitor
         RefPtr<StructTypeLayout> structTypeLayout = bufferLayout->elementTypeLayout.As<StructTypeLayout>();
         SLANG_RELEASE_ASSERT(structTypeLayout);
 
-        if( auto constantBufferType = parameterGroupType->As<ConstantBufferType>() )
-        {
-            Emit("cbuffer ");
-        }
-        else if( auto textureBufferType = parameterGroupType->As<TextureBufferType>() )
-        {
-            Emit("tbuffer ");
-        }
 
+        Emit(" ");
         if( auto reflectionNameModifier = varDecl->FindModifier<ParameterGroupReflectionName>() )
         {
-            Emit(" ");
             emitName(reflectionNameModifier->nameAndLoc);
         }
+        else
+        {
+            emitName(varDecl->nameAndLoc);
+        }
 
         EmitSemantics(varDecl, kESemanticMask_None);
 
@@ -4144,6 +4223,28 @@ emitDeclImpl(decl, nullptr);
         }
     }
 
+    // Utility code for generating unique IDs as needed
+    // during the emit process (e.g., for declarations
+    // that didn't origianlly have names, but now need to).
+
+    UInt allocateUniqueID()
+    {
+        return context->shared->uniqueIDCounter++;
+    }
+
+    UInt getID(Decl* decl)
+    {
+        auto& mapDeclToID = context->shared->mapDeclToID;
+
+        UInt id = 0;
+        if(mapDeclToID.TryGetValue(decl, id))
+            return id;
+
+        id = allocateUniqueID();
+        mapDeclToID.Add(decl, id);
+        return id;
+    }
+
     // IR-level emit logc
 
     UInt getID(IRValue* value)
@@ -4154,7 +4255,7 @@ emitDeclImpl(decl, nullptr);
         if (mapIRValueToID.TryGetValue(value, id))
             return id;
 
-        id = mapIRValueToID.Count() + 1;
+        id = allocateUniqueID();
         mapIRValueToID.Add(value, id);
         return id;
     }
@@ -6274,11 +6375,53 @@ emitDeclImpl(decl, nullptr);
         }
     }
 
+    void emitHLSLParameterBlock(
+        EmitContext*        ctx,
+        IRGlobalVar*        varDecl,
+        ParameterBlockType* type)
+    {
+        emit("cbuffer ");
+
+        // Generate a dummy name for the block
+        emit("_S");
+        Emit(ctx->shared->uniqueIDCounter++);
+
+        auto layout = getVarLayout(ctx, varDecl);
+        assert(layout);
+
+        auto info = layout->FindResourceInfo(LayoutResourceKind::ConstantBuffer);
+        SLANG_RELEASE_ASSERT(info);
+        emitHLSLRegisterSemantic(*info);
+
+        emit("\n{\n");
+
+        auto elementType = type->getElementType();
+
+        auto typeLayout = layout->typeLayout;
+        if( auto parameterGroupTypeLayout = typeLayout.As<ParameterGroupTypeLayout>() )
+        {
+            typeLayout = parameterGroupTypeLayout->elementTypeLayout;
+        }
+
+        emitIRType(ctx, elementType, getIRName(varDecl));
+
+        emitHLSLParameterGroupFieldLayoutSemantics(nullptr, layout);
+        emit(";\n");
+
+        emit("}\n");
+    }
+
     void emitHLSLParameterGroup(
         EmitContext*                ctx,
         IRGlobalVar*                varDecl,
         UniformParameterGroupType*  type)
     {
+        if(auto parameterBlockType = type->As<ParameterBlockType>())
+        {
+            emitHLSLParameterBlock(ctx, varDecl, parameterBlockType);
+            return;
+        }
+
         emit("cbuffer ");
         emit(getIRName(varDecl));
 
diff --git a/source/slang/ir-legalize-types.cpp b/source/slang/ir-legalize-types.cpp
index 6e909106d..fedec4f87 100644
--- a/source/slang/ir-legalize-types.cpp
+++ b/source/slang/ir-legalize-types.cpp
@@ -828,7 +828,7 @@ static LegalVal declareSimpleVar(
         {
             auto globalVar = builder->createGlobalVar(type);
             globalVar->removeFromParent();
-            globalVar->insertBefore(context->insertBeforeGlobal);
+            globalVar->insertBefore(context->insertBeforeGlobal, builder->getModule());
 
             irVar = globalVar;
             legalVarVal = LegalVal::simple(irVar);
diff --git a/source/slang/ir.cpp b/source/slang/ir.cpp
index d35ffe2d1..d4d2f0f51 100644
--- a/source/slang/ir.cpp
+++ b/source/slang/ir.cpp
@@ -956,16 +956,22 @@ namespace Slang
     IRInst* IRBuilder::emitLoad(
         IRValue*    ptr)
     {
-        auto ptrType = ptr->getType()->As<PtrTypeBase>();
-        if( !ptrType )
+        RefPtr<Type> valueType;
+        if(auto ptrType = ptr->getType()->As<PtrTypeBase>())
+        {
+            valueType = ptrType->getValueType();
+        }
+        else if(auto ptrLikeType = ptr->getType()->As<PointerLikeType>())
+        {
+            valueType = ptrLikeType->getElementType();
+        }
+        else
         {
             // Bad!
             SLANG_ASSERT(ptrType);
             return nullptr;
         }
 
-        auto valueType = ptrType->getValueType();
-
         auto inst = createInst<IRLoad>(
             this,
             kIROp_Load,
diff --git a/source/slang/legalize-types.cpp b/source/slang/legalize-types.cpp
index 510b9acd3..fb9c70c97 100644
--- a/source/slang/legalize-types.cpp
+++ b/source/slang/legalize-types.cpp
@@ -194,9 +194,23 @@ struct TupleTypeBuilder
             {
                 // The field's type had both special and non-special parts
                 auto pairType = legalLeafType.getPair();
-                ordinaryType = pairType->ordinaryType;
-                specialType = pairType->specialType;
-                elementPairInfo = pairType->pairInfo;
+
+                // If things originally started as a resource type, then
+                // we want to externalize all the fields that arose, even
+                // if there is (nominally) ordinary data.
+                //
+                // This is because the "ordinary" side of the legalization
+                // of `ConstantBuffer<Foo>` will still be a resource type.
+                if(isResource)
+                {
+                    specialType = legalFieldType;
+                }
+                else
+                {
+                    ordinaryType = pairType->ordinaryType;
+                    specialType = pairType->specialType;
+                    elementPairInfo = pairType->pairInfo;
+                }
             }
             break;
 
@@ -777,23 +791,21 @@ LegalType legalizeType(
     TypeLegalizationContext*    context,
     Type*                       type)
 {
-    if (auto parameterBlockType = type->As<ParameterBlockType>())
-    {
-        // We basically legalize the `ParameterBlock<T>` type
-        // over to `T`. In order to represent this preoperly,
-        // we need to be careful to wrap it up in a way that
-        // tells us to eliminate downstream deferences...
-
-        auto legalElementType = legalizeType(context,
-            parameterBlockType->getElementType());
-        return LegalType::implicitDeref(legalElementType);
-    }
-    else if (auto uniformBufferType = type->As<UniformParameterGroupType>())
+    if (auto uniformBufferType = type->As<UniformParameterGroupType>())
     {
-        // We have a `ConstantBuffer<T>` or `TextureBuffer<T>` or
-        // other pointer-like type that represents uniform parameters.
-        // We need to pull any resource-type fields out of it, but
-        // leave the non-resource fields where they are.
+        // We have one of:
+        //
+        //      ConstantBuffer<T>
+        //      TextureBuffer<T>
+        //      ParameterBlock<T>
+        //
+        // or some other pointer-like type that represents uniform
+        // parameters. We need to pull any resource-type fields out
+        // of it, but leave non-resource fields where they are.
+        //
+        // As a special case, if the type contains *no* uniform data,
+        // we'll want to completely eliminate the uniform/ordinary
+        // part.
 
         // Legalize the element type to see what we are working with.
         auto legalElementType = legalizeType(context,
@@ -973,11 +985,23 @@ RefPtr<VarLayout> getFieldLayout(
     if (!typeLayout)
         return nullptr;
 
-    while(auto arrayTypeLayout = dynamic_cast<ArrayTypeLayout*>(typeLayout))
+    for(;;)
     {
-        typeLayout = arrayTypeLayout->elementTypeLayout;
+        if(auto arrayTypeLayout = dynamic_cast<ArrayTypeLayout*>(typeLayout))
+        {
+            typeLayout = arrayTypeLayout->elementTypeLayout;
+        }
+        else if(auto parameterGroupTypeLayotu = dynamic_cast<ParameterGroupTypeLayout*>(typeLayout))
+        {
+            typeLayout = parameterGroupTypeLayotu->elementTypeLayout;
+        }
+        else
+        {
+            break;
+        }
     }
 
+
     if (auto structTypeLayout = dynamic_cast<StructTypeLayout*>(typeLayout))
     {
         RefPtr<VarLayout> fieldLayout;
diff --git a/source/slang/reflection.cpp b/source/slang/reflection.cpp
index a1ea3ff72..5962bce96 100644
--- a/source/slang/reflection.cpp
+++ b/source/slang/reflection.cpp
@@ -102,6 +102,10 @@ SLANG_API SlangTypeKind spReflectionType_GetKind(SlangReflectionType* inType)
     {
         return SLANG_TYPE_KIND_MATRIX;
     }
+    else if (auto parameterBlockType = type->As<ParameterBlockType>())
+    {
+        return SLANG_TYPE_KIND_PARAMETER_BLOCK;
+    }
     else if (auto constantBufferType = type->As<ConstantBufferType>())
     {
         return SLANG_TYPE_KIND_CONSTANT_BUFFER;
diff --git a/source/slang/syntax.h b/source/slang/syntax.h
index e6a010a4a..749c5ae32 100644
--- a/source/slang/syntax.h
+++ b/source/slang/syntax.h
@@ -38,6 +38,11 @@ namespace Slang
         // No conversion at all
         kConversionCost_None = 0,
 
+        // Conversion from a buffer to the type it carries needs to add a minimal
+        // extra cost, just so we can distinguish an overload on `ConstantBuffer<Foo>`
+        // from one on `Foo`
+        kConversionCost_ImplicitDereference = 10,
+
         // Conversions based on explicit sub-typing relationships are the cheapest
         //
         // TODO(tfoley): We will eventually need a discipline for ranking
diff --git a/source/slang/type-defs.h b/source/slang/type-defs.h
index da6e27d17..d0b2ebac1 100644
--- a/source/slang/type-defs.h
+++ b/source/slang/type-defs.h
@@ -298,7 +298,7 @@ SIMPLE_SYNTAX_CLASS(GLSLOutputParameterGroupType, VaryingParameterGroupType)
 SIMPLE_SYNTAX_CLASS(GLSLShaderStorageBufferType, UniformParameterGroupType)
 
 // type for Slang `ParameterBlock<T>` type
-SIMPLE_SYNTAX_CLASS(ParameterBlockType, ParameterGroupType)
+SIMPLE_SYNTAX_CLASS(ParameterBlockType, UniformParameterGroupType)
 
 SYNTAX_CLASS(ArrayExpressionType, Type)
     SYNTAX_FIELD(RefPtr<Type>, baseType)