Add support for default parameter values in IR codegen (shader-slang#459)

Fixes shader-slang#61

When lowering from AST to IR, if a call site doesn't supply an argument expression for each of the parameters to the callee, then use the default value expressions (stored as the "initializer" of the parameter decl) for each omitted parameter. This relies on the front-end to have already checked the call site for validity.

Along the way I also cleaned up some of the checking of parameter declarations so that it is more like the checking of ordinary variable declarations (although the code is not yet shared). I also cleaned out some dead cases in the lowering logic for when we don't actually have a declaration available for a callee (these would only matter if we supported functions as first-class values).

I added a simple test case to confirm that call sites both with and without the optional parameter work as expected.

The strategy in this change is extremely simplistic, and might only be appropriate for default parameter value expressions that are compile-time constants (which should be the 99% case). This may require a major overhaul if we decide to handle default parameter values differently (e.g., by generating extra functions to ensure that the separate compilation story is what we want).

Another issue that could change a lot of this logic would be if we start to support by-name parameters at call sites, since we could no longer assume that the argument and parameter lists align one-to-one (with the argument list possibly being shorter). Any work to add more flexible argument passing conventions would need to build a suitable structure to map from arguments to parameters, or vice-versa.

Author: Tim Foley

source/slang/check.cpp

@@ -2820,18 +2820,38 @@ namespace Slang
             // Nothing to do
         }
 
-        void visitParamDecl(ParamDecl* para)
+        void visitParamDecl(ParamDecl* paramDecl)
         {
-            // TODO: This needs to bottleneck through the common variable checks
+            // TODO: This logic should be shared with the other cases of
+            // variable declarations. The main reason I am not doing it
+            // yet is that we use a `ParamDecl` with a null type as a
+            // special case in attribute declarations, and that could
+            // trip up the ordinary variable checks.
 
-            if(para->type.exp)
+            auto typeExpr = paramDecl->type;
+            if(typeExpr.exp)
             {
-                para->type = CheckUsableType(para->type);
-            
-                if (para->type.Equals(getSession()->getVoidType()))
-                {
-                    getSink()->diagnose(para, Diagnostics::parameterCannotBeVoid);
-                }
+                typeExpr = CheckUsableType(typeExpr);
+                paramDecl->type = typeExpr;
+            }
+
+            // The "initializer" expression for a parameter represents
+            // a default argument value to use if an explicit one is
+            // not supplied.
+            if(auto initExpr = paramDecl->initExpr)
+            {
+                // We must check the expression and coerce it to the
+                // actual type of the parameter.
+                //
+                initExpr = CheckExpr(initExpr);
+                initExpr = Coerce(typeExpr.type, initExpr);
+                paramDecl->initExpr = initExpr;
+
+                // TODO: a default argument expression needs to
+                // conform to other constraints to be valid.
+                // For example, it should not be allowed to refer
+                // to other parameters of the same function (or maybe
+                // only the parameters to its left...).
             }
         }
 

source/slang/diagnostic-defs.h

@@ -176,7 +176,6 @@ DIAGNOSTIC(30013, Error, subscriptNonArray, "no subscript operation found for  t
 DIAGNOSTIC(30014, Error, subscriptIndexNonInteger, "index expression must evaluate to int.")
 DIAGNOSTIC(30015, Error, undefinedIdentifier, "'$0': undefined identifier.")
 DIAGNOSTIC(30015, Error, undefinedIdentifier2, "undefined identifier '$0'.")
-DIAGNOSTIC(30016, Error, parameterCannotBeVoid, "'void' can not be parameter type.")
 DIAGNOSTIC(30017, Error, componentNotAccessibleFromShader, "component '$0' is not accessible from shader '$1'.")
 DIAGNOSTIC(30019, Error, typeMismatch, "expected an expression of type '$0', got '$1'")
 DIAGNOSTIC(30020, Error, importOperatorReturnTypeMismatch, "import operator should return '$1', but the expression has type '$0''. do you forget 'project'?")

source/slang/lower-to-ir.cpp

@@ -1417,21 +1417,6 @@ struct ExprLoweringVisitorBase : ExprVisitor<Derived, LoweredValInfo>
         SLANG_UNIMPLEMENTED_X("codegen for aggregate type constructor expression");
     }
 
-    // Add arguments that appeared directly in an argument list
-    // to the list of argument values for a call.
-    void addDirectCallArgs(
-        InvokeExpr*     expr,
-        List<IRInst*>* ioArgs)
-    {
-        for( auto arg : expr->Arguments )
-        {
-            // TODO: Need to handle case of l-value arguments,
-            // when they are matched to `out` or `in out` parameters.
-            auto loweredArg = lowerRValueExpr(context, arg);
-            addArgs(context, ioArgs, loweredArg);
-        }
-    }
-
     // After a call to a function with `out` or `in out`
     // parameters, we may need to copy data back into
     // the l-value locations used for output arguments.
@@ -1452,18 +1437,44 @@ struct ExprLoweringVisitorBase : ExprVisitor<Derived, LoweredValInfo>
         List<OutArgumentFixup>* ioFixups)
     {
         UInt argCount = expr->Arguments.Count();
-        UInt argIndex = 0;
+        UInt argCounter = 0;
         for (auto paramDeclRef : getMembersOfType<ParamDecl>(funcDeclRef))
         {
-            if (argIndex >= argCount)
-            {
-                // The remaining parameters must be defaulted...
-                break;
-            }
-
             auto paramDecl = paramDeclRef.getDecl();
             RefPtr<Type> paramType = lowerSimpleType(context, GetType(paramDeclRef));
-            auto argExpr = expr->Arguments[argIndex++];
+
+            UInt argIndex = argCounter++;
+            RefPtr<Expr> argExpr;
+            if(argIndex < argCount)
+            {
+                argExpr = expr->Arguments[argIndex];
+            }
+            else
+            {
+                // We have run out of arguments supplied at the call site,
+                // but there are still parameters remaining. This must mean
+                // that these parameters have default argument expressions
+                // associated with them.
+                argExpr = getInitExpr(paramDeclRef);
+
+                // Assert that such an expression must have been present.
+                SLANG_ASSERT(argExpr);
+
+                // TODO: The approach we are taking here to default arguments
+                // is simplistic, and has consequences for the front-end as
+                // well as binary serializatiojn of modules.
+                //
+                // We could consider some more refined approaches where, e.g.,
+                // functions with default arguments generate multiple IR-level
+                // functions, that compute and provide the default values.
+                //
+                // Alternatively, each parameter with defaults could be generated
+                // into its own callable function that provides the default value,
+                // so that calling modules can call into a pre-generated function.
+                //
+                // Each of these options involves trade-offs, and we need to
+                // make a conscious decision at some point.
+            }
 
             if (paramDecl->HasModifier<OutModifier>()
                 || paramDecl->HasModifier<InOutModifier>())
@@ -1543,8 +1554,7 @@ struct ExprLoweringVisitorBase : ExprVisitor<Derived, LoweredValInfo>
         }
         else
         {
-            SLANG_UNEXPECTED("shouldn't relaly happen");
-            UNREACHABLE(addDirectCallArgs(expr, ioArgs));
+            SLANG_UNEXPECTED("callee was not a callable decl");
         }
     }
 
@@ -1696,24 +1706,21 @@ struct ExprLoweringVisitorBase : ExprVisitor<Derived, LoweredValInfo>
             return result;
         }
 
-        // The default case is to assume that we just have
-        // an ordinary expression, and can lower it as such.
-        LoweredValInfo funcVal = lowerRValueExpr(context, expr->FunctionExpr);
-
-        // Now we add any direct arguments from the call expression itself.
-        addDirectCallArgs(expr, &irArgs);
-
-        // Delegate to the logic for invoking a value.
-        auto result = emitCallToVal(context, type, funcVal, irArgs.Count(), irArgs.Buffer());
-
-        // TODO: because of the nature of how the `emitCallToVal` case works
-        // right now, we don't have information on in/out parameters, and
-        // so we can't collect info to apply fixups.
+        // TODO: In this case we should be emitting code for the callee as
+        // an ordinary expression, then emitting the arguments according
+        // to the type information on the callee (e.g., which paameters
+        // are `out` or `inout`, and then finally emitting the `call`
+        // instruciton.
         //
-        // Once we have a better representation for function types, though,
-        // this should be fixable.
-
-        return result;
+        // We don't currently have the case of emitting arguments according
+        // to function type info (instead of declaration info), and really
+        // this case can't occur unless we start adding first-class functions
+        // to the source language.
+        //
+        // For now we just bail out with an error.
+        //
+        SLANG_UNEXPECTED("could not resolve target declaration for call");
+        UNREACHABLE_RETURN(LoweredValInfo());
     }
 
     LoweredValInfo subscriptValue(

tests/compute/default-parameter.slang

@@ -0,0 +1,24 @@
+//TEST(compute):COMPARE_COMPUTE_EX:-slang -compute
+//TEST(compute, vulkan):COMPARE_COMPUTE_EX:-vk -compute
+
+//TEST_INPUT:ubuffer(data=[0 0 0 0], stride=4):dxbinding(0),glbinding(0),out
+
+RWStructuredBuffer<int> outputBuffer;
+
+int helper(int val, int a = 16)
+{
+    return val + a;
+}
+
+int test(int val)
+{
+    return helper(val) + helper(val, 256);
+}
+
+[numthreads(4, 1, 1)]
+void computeMain(uint3 dispatchThreadID : SV_DispatchThreadID)
+{
+    int inVal = (int) dispatchThreadID.x;
+    int outVal = test(inVal);
+    outputBuffer[dispatchThreadID.x] = outVal;
+}

tests/compute/default-parameter.slang.1.expected.txt

@@ -0,0 +1,4 @@
+110
+112
+114
+116

tests/compute/default-parameter.slang.expected.txt

@@ -0,0 +1,4 @@
+110
+112
+114
+116