A small refactor to how implicit constant buffers are getting created. (#871)

This affects layout computation for both the global and entry-point scopes, where multiple discrete shader parameters can be declared, but for layout purposes they must be treated as if they lived in the same `struct` type. If that `struct` type ends up consuming any "ordinary" data (`LayoutResourceKind::Uniform`) then an implicit constant buffer will be needed for that scope (e.g., the way fxc produces a `$Globals` constant buffer for the global scope).

The logic for computing those scope layouts had a bug in it, in that the struct type was not being updated to have the right size for uniform data at the scope. That bug hasn't bitten anybody yet because no Slang users are relying on entry-point uniforms, and global-scope uniforms aren't fully implemented (and get diagnosed as an error elsewhere in the compiler). This change fixes that bug.

This change also refactors things so that the logic for creating a constant buffer layout if and only if needed is moved into `type-layout.cpp` instead of relying on `parameter-binding.cpp` to compute whether or not it needs a block on its own. This is anticipating the rules for deciding whether or not a constant buffer is needed being slightly more thorny once interface types are in the mix.

3 files changed, 52 insertions, 31 deletions

diff --git a/source/slang/parameter-binding.cpp b/source/slang/parameter-binding.cpp
index f0256164c..dda3a8621 100644
--- a/source/slang/parameter-binding.cpp
+++ b/source/slang/parameter-binding.cpp
@@ -2345,7 +2345,6 @@ struct ScopeLayoutBuilder
     LayoutRulesImpl*            m_rules = nullptr;
     RefPtr<StructTypeLayout>    m_structLayout;
     UniformLayoutInfo           m_structLayoutInfo;
-    bool                        m_needConstantBuffer = false;
 
     void beginLayout(
         ParameterBindingContext* context)
@@ -2367,8 +2366,6 @@ struct ScopeLayoutBuilder
         LayoutSize uniformSize = layoutInfo ? layoutInfo->count : 0;
         if( uniformSize != 0 )
         {
-            m_needConstantBuffer = true;
-
             // Make sure uniform fields get laid out properly...
 
             UniformLayoutInfo fieldInfo(
@@ -2424,25 +2421,25 @@ struct ScopeLayoutBuilder
 
     RefPtr<VarLayout> endLayout()
     {
+        // Finish computing the layout for the ordindary data (if any).
+        //
         m_rules->EndStructLayout(&m_structLayoutInfo);
 
+        // Copy the final layout information computed for ordinary data
+        // over to the struct type layout for the scope.
+        //
+        m_structLayout->addResourceUsage(LayoutResourceKind::Uniform, m_structLayoutInfo.size);
+        m_structLayout->uniformAlignment = m_structLayout->uniformAlignment;
+
         RefPtr<TypeLayout> scopeTypeLayout = m_structLayout;
 
-        // If the caller decided to allocate a constant buffer for
-        // the ordinary data, then we need to wrap up the structure
-        // type (layout) in a constant buffer type (layout).
+        // If a constant buffer is needed (because there is a non-zero
+        // amount of uniform data), then we need to wrap up the layout
+        // to reflect the constant buffer that will be generated.
         //
-        if( m_needConstantBuffer )
-        {
-            auto constantBufferLayout = createParameterGroupTypeLayout(
-                m_context->layoutContext,
-                nullptr,
-                m_rules,
-                m_rules->GetObjectLayout(ShaderParameterKind::ConstantBuffer),
-                m_structLayout);
-
-            scopeTypeLayout = constantBufferLayout;
-        }
+        scopeTypeLayout = createConstantBufferTypeLayoutIfNeeded(
+            m_context->layoutContext,
+            scopeTypeLayout);
 
         // We now have a bunch of layout information, which we should
         // record into a suitable object that represents the scope
diff --git a/source/slang/type-layout.cpp b/source/slang/type-layout.cpp
index eefcfe1fd..ea5287999 100644
--- a/source/slang/type-layout.cpp
+++ b/source/slang/type-layout.cpp
@@ -1378,20 +1378,41 @@ static RefPtr<ParameterGroupTypeLayout> _createParameterGroupTypeLayout(
     return typeLayout;
 }
 
-RefPtr<ParameterGroupTypeLayout> createParameterGroupTypeLayout(
+    /// Doe we need to wrap the given element type in a constant buffer layout?
+static bool needsConstantBuffer(RefPtr<TypeLayout> elementTypeLayout)
+{
+    auto uniformResInfo = elementTypeLayout->FindResourceInfo(LayoutResourceKind::Uniform);
+    if(uniformResInfo && uniformResInfo->count != 0)
+        return true;
+
+    // Note: additional cases are expected here, to deal with existential types.
+
+    return false;
+}
+
+RefPtr<TypeLayout> createConstantBufferTypeLayoutIfNeeded(
     TypeLayoutContext const&    context,
-    RefPtr<ParameterGroupType>  parameterGroupType,
-    LayoutRulesImpl*            parameterGroupRules,
-    SimpleLayoutInfo            parameterGroupInfo,
     RefPtr<TypeLayout>          elementTypeLayout)
 {
+    // First things first, we need to check whether the element type
+    // we are trying to lay out even needs a constant buffer allocated
+    // for it.
+    //
+    if(!needsConstantBuffer(elementTypeLayout))
+        return elementTypeLayout;
+
+    auto parameterGroupRules = context.getRulesFamily()->getConstantBufferRules();
+
     return _createParameterGroupTypeLayout(
-        context.with(parameterGroupRules).with(context.targetReq->getDefaultMatrixLayoutMode()),
-        parameterGroupType,
-        parameterGroupInfo,
+        context
+            .with(parameterGroupRules)
+            .with(context.targetReq->getDefaultMatrixLayoutMode()),
+        nullptr,
+        parameterGroupRules->GetObjectLayout(ShaderParameterKind::ConstantBuffer),
         elementTypeLayout);
 }
 
+
 static RefPtr<ParameterGroupTypeLayout> _createParameterGroupTypeLayout(
     TypeLayoutContext const&    context,
     RefPtr<ParameterGroupType>  parameterGroupType,
diff --git a/source/slang/type-layout.h b/source/slang/type-layout.h
index b3b1ed1df..18a51c155 100644
--- a/source/slang/type-layout.h
+++ b/source/slang/type-layout.h
@@ -924,16 +924,19 @@ RefPtr<ParameterGroupTypeLayout> createParameterGroupTypeLayout(
     TypeLayoutContext const&    context,
     RefPtr<ParameterGroupType>  parameterGroupType);
 
-    /// Create a layout for a parameter-group type (a `ConstantBuffer` or `ParameterBlock`).
+    /// Create a wrapper constant buffer type layout, if needed.
     ///
-    /// This overload allows the `parameterGroupType` parameter to be null, for cases
-    /// where an anonymous parameter group needs to be constructed.
+    /// When dealing with entry-point `uniform` and global-scope parameters,
+    /// we want to create a wrapper constant buffer for all the parameters
+    /// if and only if there exist some parameters that use "ordinary" data
+    /// (`LayoutResourceKind::Uniform`).
     ///
-RefPtr<ParameterGroupTypeLayout> createParameterGroupTypeLayout(
+    /// This function determines whether such a wrapper is needed, based
+    /// on the `elementTypeLayout` given, and either creates and returns
+    /// the layout for the wrapper, or the unmodified `elementTypeLayout`.
+    ///
+RefPtr<TypeLayout> createConstantBufferTypeLayoutIfNeeded(
     TypeLayoutContext const&    context,
-    RefPtr<ParameterGroupType>  parameterGroupType,
-    LayoutRulesImpl*            parameterGroupRules,
-    SimpleLayoutInfo            parameterGroupInfo,
     RefPtr<TypeLayout>          elementTypeLayout);
 
 // Create a type layout for a structured buffer type.