Cleanups to ParameterBlock<T> behavior. (shader-slang#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

Author: Tim Foley

slang.h

@@ -522,6 +522,7 @@ extern "C"
         SLANG_TYPE_KIND_SAMPLER_STATE,
         SLANG_TYPE_KIND_TEXTURE_BUFFER,
         SLANG_TYPE_KIND_SHADER_STORAGE_BUFFER,
+        SLANG_TYPE_KIND_PARAMETER_BLOCK,
 
         SLANG_TYPE_KIND_COUNT,
     };
@@ -759,6 +760,7 @@ namespace slang
             SamplerState = SLANG_TYPE_KIND_SAMPLER_STATE,
             TextureBuffer = SLANG_TYPE_KIND_TEXTURE_BUFFER,
             ShaderStorageBuffer = SLANG_TYPE_KIND_SHADER_STORAGE_BUFFER,
+            ParameterBlock = SLANG_TYPE_KIND_PARAMETER_BLOCK,
         };
 
         enum ScalarType : SlangScalarType

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(

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();