Fix atomic operations on RWBuffer (#593)

* Fix atomic operations on RWBuffer

An earlier change added support for passing true pointers to `__ref` parameters to fix the global `Interlocked*()` functions when applied to `groupshared` variables or `RWStructureBuffer<T>` elements.
That change didn't apply to `RWBuffer<T>` or `RWTexture2D<T>`, etc. because those types had so far only declared `get` and `set` accessors, but not any `ref` accessors (which return a pointer).

The main fixes here are:

* Add `ref` accessors to the subscript oeprations on the `RW*` resource types

* Adjust the logic for emitting calls to subscript accessors so that we don't get quite as eager about invoking a `ref` accessor, and instead try to invoke just a `get` or `set` accessor when these will suffice. This is important for Vulkan cross-compilation, where we don't yet support the semantics of our `ref` accessors.

* Add a test case for atomics on a `RWBuffer`

* Fix up `render-test` so that we can specify a format for a buffer resource, which allows us to use things other than `*StructuredBuffer` and `*ByteAddressBuffer`. The work there is probably not complete; I just did what I could to get the test working.

* A bunch of files got whitespace edits thanks to the fact that I'm using editorconfig and others on the project seemingly arent...

* fixup: remove ifdefed-out code

6 files changed, 121 insertions, 46 deletions

diff --git a/source/slang/core.meta.slang b/source/slang/core.meta.slang
index 00afde2ac..507ae014e 100644
--- a/source/slang/core.meta.slang
+++ b/source/slang/core.meta.slang
@@ -326,7 +326,7 @@ for( int C = 2; C <= 4; ++C )
 sb << "__magic_type(SamplerState," << int(SamplerStateFlavor::SamplerState) << ")\n";
 sb << "__intrinsic_type(" << kIROp_SamplerStateType << ")\n";
 sb << "struct SamplerState {};";
-        
+
 sb << "__magic_type(SamplerState," << int(SamplerStateFlavor::SamplerComparisonState) << ")\n";
 sb << "__intrinsic_type(" << kIROp_SamplerComparisonStateType << ")\n";
 sb << "struct SamplerComparisonState {};";
@@ -688,6 +688,11 @@ for (int tt = 0; tt < kBaseTextureTypeCount; ++tt)
 
                 default:
                     sb << "__target_intrinsic(glsl, \"imageStore($0, " << ivecN << "($1), $2)\") set;\n";
+
+                    // TODO: really need a way to map access through `ref` accessor (e.g., when
+                    // used with an atomic operation) over to GLSL equivalent.
+                    //
+                    sb << "ref;\n";
                     break;
                 }
 
@@ -911,7 +916,7 @@ for (int tt = 0; tt < kBaseTextureTypeCount; ++tt)
                     auto componentName = kk.componentName;
 
                     EMIT_LINE_DIRECTIVE();
-                            
+
                     sb << "__target_intrinsic(glsl, \"textureGather($p, $2, " << componentIndex << ")\")\n";
                     sb << "vector<T, 4> Gather" << componentName << "(SamplerState s, ";
                     sb << "float" << kBaseTextureTypes[tt].coordCount << " location);\n";
diff --git a/source/slang/core.meta.slang.h b/source/slang/core.meta.slang.h
index 95c6ff0f7..948ca4207 100644
--- a/source/slang/core.meta.slang.h
+++ b/source/slang/core.meta.slang.h
@@ -341,7 +341,7 @@ for( int C = 2; C <= 4; ++C )
 sb << "__magic_type(SamplerState," << int(SamplerStateFlavor::SamplerState) << ")\n";
 sb << "__intrinsic_type(" << kIROp_SamplerStateType << ")\n";
 sb << "struct SamplerState {};";
-        
+
 sb << "__magic_type(SamplerState," << int(SamplerStateFlavor::SamplerComparisonState) << ")\n";
 sb << "__intrinsic_type(" << kIROp_SamplerComparisonStateType << ")\n";
 sb << "struct SamplerComparisonState {};";
@@ -703,6 +703,11 @@ for (int tt = 0; tt < kBaseTextureTypeCount; ++tt)
 
                 default:
                     sb << "__target_intrinsic(glsl, \"imageStore($0, " << ivecN << "($1), $2)\") set;\n";
+
+                    // TODO: really need a way to map access through `ref` accessor (e.g., when
+                    // used with an atomic operation) over to GLSL equivalent.
+                    //
+                    sb << "ref;\n";
                     break;
                 }
 
@@ -926,7 +931,7 @@ for (int tt = 0; tt < kBaseTextureTypeCount; ++tt)
                     auto componentName = kk.componentName;
 
                     EMIT_LINE_DIRECTIVE();
-                            
+
                     sb << "__target_intrinsic(glsl, \"textureGather($p, $2, " << componentIndex << ")\")\n";
                     sb << "vector<T, 4> Gather" << componentName << "(SamplerState s, ";
                     sb << "float" << kBaseTextureTypes[tt].coordCount << " location);\n";
diff --git a/source/slang/emit.cpp b/source/slang/emit.cpp
index e0f71aafe..3f88ab82d 100644
--- a/source/slang/emit.cpp
+++ b/source/slang/emit.cpp
@@ -2363,15 +2363,18 @@ struct EmitVisitor
         // for temporary variables.
         auto type = inst->getDataType();
 
-        // First we unwrap any layers of pointer-ness and array-ness
-        // from the types to get at the underlying data type.
-        while (auto ptrType = as<IRPtrTypeBase>(type))
+        // Unwrap any layers of array-ness from the type, so that
+        // we can look at the underlying data type, in case we
+        // should *never* expose a value of that type
+        while (auto arrayType = as<IRArrayTypeBase>(type))
         {
-            type = ptrType->getValueType();
+            type = arrayType->getElementType();
         }
-        while (auto ptrType = as<IRArrayTypeBase>(type))
+
+        // Don't allow temporaries of pointer types to be created.
+        if(as<IRPtrTypeBase>(type))
         {
-            type = ptrType->getElementType();
+            return true;
         }
 
         // First we check for uniform parameter groups,
diff --git a/source/slang/hlsl.meta.slang b/source/slang/hlsl.meta.slang
index 152e9faa1..86f9eb946 100644
--- a/source/slang/hlsl.meta.slang
+++ b/source/slang/hlsl.meta.slang
@@ -1144,7 +1144,7 @@ for (int aa = 0; aa < kBaseBufferAccessLevelCount; ++aa)
 
     if (kBaseBufferAccessLevels[aa].access != SLANG_RESOURCE_ACCESS_READ)
     {
-        sb << "set;\n";
+        sb << "ref;\n";
     }
 
     sb << "}\n";
diff --git a/source/slang/hlsl.meta.slang.h b/source/slang/hlsl.meta.slang.h
index ba4998b23..c05e81cd1 100644
--- a/source/slang/hlsl.meta.slang.h
+++ b/source/slang/hlsl.meta.slang.h
@@ -1177,7 +1177,7 @@ for (int aa = 0; aa < kBaseBufferAccessLevelCount; ++aa)
 
     if (kBaseBufferAccessLevels[aa].access != SLANG_RESOURCE_ACCESS_READ)
     {
-        sb << "set;\n";
+        sb << "ref;\n";
     }
 
     sb << "}\n";
diff --git a/source/slang/lower-to-ir.cpp b/source/slang/lower-to-ir.cpp
index 96b28a15b..12ba94146 100644
--- a/source/slang/lower-to-ir.cpp
+++ b/source/slang/lower-to-ir.cpp
@@ -573,33 +573,8 @@ LoweredValInfo emitCallToDeclRef(
         bool justAGetter = true;
         for (auto accessorDeclRef : getMembersOfType<AccessorDecl>(subscriptDeclRef))
         {
-            // If the subscript declares a `ref` accessor, then we can just
-            // invoke that directly to get an l-value we can use.
-            if(auto refAccessorDeclRef = accessorDeclRef.As<RefAccessorDecl>())
-            {
-                // The `ref` accessor will return a pointer to the value, so
-                // we need to reflect that in the type of our `call` instruction.
-                IRType* ptrType = context->irBuilder->getPtrType(type);
-
-                // Rather than call `emitCallToVal` here, we make a recursive call
-                // to `emitCallToDeclRef` so that it can handle things like intrinsic-op
-                // modifiers attached to the acecssor.
-                LoweredValInfo callVal = emitCallToDeclRef(
-                    context,
-                    ptrType,
-                    refAccessorDeclRef,
-                    funcType,
-                    argCount,
-                    args);
-
-                // The result from the call needs to be implicitly dereferenced,
-                // so that it can work as an l-value of the desired result type.
-                return LoweredValInfo::ptr(getSimpleVal(context, callVal));
-            }
-
-            // If we don't find a `ref` accessor, then we want to track whether
-            // this subscript has any accessors other than `get` (assuming
-            // that everything except `get` can be used for setting...).
+            // We want to track whether this subscript has any accessors other than
+            // `get` (assuming that everything except `get` can be used for setting...).
 
             if (auto foundGetterDeclRef = accessorDeclRef.As<GetterDecl>())
             {
@@ -821,6 +796,15 @@ top:
         {
             auto boundSubscriptInfo = lowered.getBoundSubscriptInfo();
 
+            // We are being asked to extract a value from a subscript call
+            // (e.g., `base[index]`). We will first check if the subscript
+            // declared a getter and use that if possible, and then fall
+            // back to a `ref` accessor if one is defined.
+            //
+            // (Picking the `get` over the `ref` accessor simplifies things
+            // in case the `get` operation has a natural translation for
+            // a target, while the general `ref` case does not...)
+
             auto getters = getMembersOfType<GetterDecl>(boundSubscriptInfo->declRef);
             if (getters.Count())
             {
@@ -833,6 +817,27 @@ top:
                 goto top;
             }
 
+            auto refAccessors = getMembersOfType<RefAccessorDecl>(boundSubscriptInfo->declRef);
+            if(refAccessors.Count())
+            {
+                // The `ref` accessor will return a pointer to the value, so
+                // we need to reflect that in the type of our `call` instruction.
+                IRType* ptrType = context->irBuilder->getPtrType(boundSubscriptInfo->type);
+
+                LoweredValInfo refVal = emitCallToDeclRef(
+                    context,
+                    ptrType,
+                    *refAccessors.begin(),
+                    nullptr,
+                    boundSubscriptInfo->args);
+
+                // The result from the call needs to be implicitly dereferenced,
+                // so that it can work as an l-value of the desired result type.
+                lowered = LoweredValInfo::ptr(getSimpleVal(context, refVal));
+
+                goto top;
+            }
+
             SLANG_UNEXPECTED("subscript had no getter");
             UNREACHABLE_RETURN(LoweredValInfo());
         }
@@ -1308,7 +1313,7 @@ static void addNameHint(
 {
     Name* name = getNameForNameHint(context, decl);
     if(!name)
-        return;    
+        return;
     context->irBuilder->addDecoration<IRNameHintDecoration>(inst)->name = name;
 }
 
@@ -2958,18 +2963,51 @@ IRInst* getAddress(
     SourceLoc               diagnosticLocation)
 {
     LoweredValInfo val = inVal;
+
     switch(val.flavor)
     {
     case LoweredValInfo::Flavor::Ptr:
         return val.val;
 
-    // TODO: are there other cases we need to handle here (e.g.,
-    // turning a bound subscript/property into an address)
+    case LoweredValInfo::Flavor::BoundSubscript:
+        {
+            // If we are are trying to turn a subscript operation like `buffer[index]`
+            // into a pointer, then we need to find a `ref` accessor declared
+            // as part of the subscript operation being referenced.
+            //
+            auto subscriptInfo = val.getBoundSubscriptInfo();
+            auto refAccessors = getMembersOfType<RefAccessorDecl>(subscriptInfo->declRef);
+            if(refAccessors.Count())
+            {
+                // The `ref` accessor will return a pointer to the value, so
+                // we need to reflect that in the type of our `call` instruction.
+                IRType* ptrType = context->irBuilder->getPtrType(subscriptInfo->type);
+
+                LoweredValInfo refVal = emitCallToDeclRef(
+                    context,
+                    ptrType,
+                    *refAccessors.begin(),
+                    nullptr,
+                    subscriptInfo->args);
+
+                // The result from the call should be a pointer, and it
+                // is the address that we wanted in the first place.
+                return getSimpleVal(context, refVal);
+            }
+
+            // Otherwise, there was no `ref` accessor, and so it is not possible
+            // to materialize this location into a pointer for whatever purpose
+            // we have in mind (e.g., passing it to an atomic operation).
+        }
+
+    // TODO: are there other cases we need to handled here?
 
     default:
-        context->getSink()->diagnose(diagnosticLocation, Diagnostics::invalidLValueForRefParameter);
-        return nullptr;
+        break;
     }
+
+    context->getSink()->diagnose(diagnosticLocation, Diagnostics::invalidLValueForRefParameter);
+    return nullptr;
 }
 
 void assign(
@@ -3090,7 +3128,10 @@ top:
             // `someStructuredBuffer[index]`.
             //
             // When storing to such a value, we need to emit a call
-            // to the appropriate builtin "setter" accessor.
+            // to the appropriate builtin "setter" accessor, if there
+            // is one, and then fall back to a `ref` accessor if
+            // there is no setter.
+            //
             auto subscriptInfo = left.getBoundSubscriptInfo();
 
             // Search for an appropriate "setter" declaration
@@ -3098,7 +3139,6 @@ top:
             if (setters.Count())
             {
                 auto allArgs = subscriptInfo->args;
-
                 addArgs(context, &allArgs, right);
 
                 emitCallToDeclRef(
@@ -3110,6 +3150,28 @@ top:
                 return;
             }
 
+            auto refAccessors = getMembersOfType<RefAccessorDecl>(subscriptInfo->declRef);
+            if(refAccessors.Count())
+            {
+                // The `ref` accessor will return a pointer to the value, so
+                // we need to reflect that in the type of our `call` instruction.
+                IRType* ptrType = context->irBuilder->getPtrType(subscriptInfo->type);
+
+                LoweredValInfo refVal = emitCallToDeclRef(
+                    context,
+                    ptrType,
+                    *refAccessors.begin(),
+                    nullptr,
+                    subscriptInfo->args);
+
+                // The result from the call needs to be implicitly dereferenced,
+                // so that it can work as an l-value of the desired result type.
+                left = LoweredValInfo::ptr(getSimpleVal(context, refVal));
+
+                // Tail-recursively attempt assignment again on the new l-value.
+                goto top;
+            }
+
             // No setter found? Then we have an error!
             SLANG_UNEXPECTED("no setter found");
             break;