From d7280c3eb0341906a24084595b1a44aed24b3eb7 Mon Sep 17 00:00:00 2001
From: jsmall-nvidia <jsmall@nvidia.com>
Date: Wed, 13 Feb 2019 14:49:02 -0500
Subject: Output readonly for suitable glsl buffers (#845)

* Output readonly on buffers for glsl if resource is readonly.
Didn't add to emitGLSLParameterGroup because the cases there seem to to either be implicitly read only, or allow write.

* * Improve comments around use of 'readonly' on glsl output
* Use readonly with shaderRecord

* Add comment pointing out shader record can be rw on vk, so might require changes in the future.
---
 source/slang/emit.cpp | 56 +++++++++++++++++++++++++++++++++++++++++++++++----
 1 file changed, 52 insertions(+), 4 deletions(-)

(limited to 'source/slang/emit.cpp')

diff --git a/source/slang/emit.cpp b/source/slang/emit.cpp
index eeb23a6e5..c0e5e4296 100644
--- a/source/slang/emit.cpp
+++ b/source/slang/emit.cpp
@@ -5857,21 +5857,33 @@ struct EmitVisitor
             typeLayout = parameterGroupTypeLayout->elementVarLayout->typeLayout;
         }
 
+        /*
+        With resources backed by 'buffer' on glsl, we want to output 'readonly' if that is a good match
+        for the underlying type. If uniform it's implicit it's readonly
+
+        Here this only happens with isShaderRecord which is a 'constant buffer' (ie implicitly readonly)
+        or IRGLSLShaderStorageBufferType which is read write.
+        */
+
         emitGLSLLayoutQualifier(LayoutResourceKind::DescriptorTableSlot, &containerChain);
         emitGLSLLayoutQualifier(LayoutResourceKind::PushConstantBuffer, &containerChain);
         bool isShaderRecord = emitGLSLLayoutQualifier(LayoutResourceKind::ShaderRecord, &containerChain);
 
         if( isShaderRecord )
         {
-            emit("buffer ");
+            // TODO: A shader record in vk can be potentially read write. Currently slang does't support write access
+            // so for now we will assume readonly
+            emit("readonly buffer ");
         }
         else if(as<IRGLSLShaderStorageBufferType>(type))
         {
+            // Is writable 
             emit("layout(std430) buffer ");
         }
         // TODO: what to do with HLSL `tbuffer` style buffers?
         else
         {
+            // uniform is implicitly read only
             emit("layout(std140) uniform ");
         }
 
@@ -5995,7 +6007,27 @@ struct EmitVisitor
             }
         }
         
-        emit(") buffer ");
+        emit(") ");
+
+        /*
+        If the output type is a buffer, and we can determine it is only readonly we can prefix before
+        buffer with 'readonly'
+
+        The actual structuredBufferType could be
+
+        HLSLStructuredBufferType                        - This is unambiguously read only
+        HLSLRWStructuredBufferType                      - Read write
+        HLSLRasterizerOrderedStructuredBufferType       - Allows read/write access
+        HLSLAppendStructuredBufferType                  - Write
+        HLSLConsumeStructuredBufferType                 - TODO (JS): Its possible that this can be readonly, but we currently don't support on GLSL
+        */
+
+        if (as<IRHLSLStructuredBufferType>(structuredBufferType))
+        {
+            emit("readonly ");
+        }
+
+        emit("buffer ");
     
         // Generate a dummy name for the block
         emit("_S");
@@ -6021,7 +6053,7 @@ struct EmitVisitor
     void emitIRByteAddressBuffer_GLSL(
         EmitContext*                    ctx,
         IRGlobalParam*                  varDecl,
-        IRByteAddressBufferTypeBase*    /* byteAddressBufferType */)
+        IRByteAddressBufferTypeBase*    byteAddressBufferType)
     {
         // TODO: A lot of this logic is copy-pasted from `emitIRStructuredBuffer_GLSL`.
         // It might be worthwhile to share the common code to avoid regressions sneaking
@@ -6052,7 +6084,23 @@ struct EmitVisitor
             }
         }
 
-        emit(") buffer ");
+        emit(") ");
+
+        /*
+        If the output type is a buffer, and we can determine it is only readonly we can prefix before
+        buffer with 'readonly'
+
+        HLSLByteAddressBufferType                   - This is unambiguously read only
+        HLSLRWByteAddressBufferType                 - Read write
+        HLSLRasterizerOrderedByteAddressBufferType  - Allows read/write access
+        */
+
+        if (as<IRHLSLByteAddressBufferType>(byteAddressBufferType))
+        {
+            emit("readonly ");
+        }
+
+        emit("buffer ");
 
         // Generate a dummy name for the block
         emit("_S");
-- 
cgit v1.2.3