IR: fixes for subscript accessors (shader-slang#322)

* IR: fixes for subscript accessors

Fixes shader-slang#320

This is a bunch of fixes for handling of `__subscript` operations on builtin types (notably `RWStructuredBuffer` and `StructuredBuffer` at this point).

- Automatically add a `GetterDecl` to any subscript decalratio was declithout any accessors. This avoids hitting a null- dereference in the emit logic.

- Add a notion of a `RefAccessor` (declared with `ref`) as a peer to getters and setters. The idea is that a `ref` accessor returns a pointer to the element data, so that it can be used for both getting and setting values. This is closer to the behavior of `RWStructuredBuffer` element access in HLSL.

- Fixes for dealing with "access chains" where there might be a combination of a subscript (where the is a `get` and `set` but no `ref`) and member access, so that we have to read the base value into a temp, modify it, and then write it back.
  - This logic is still a bit of a mess, so we will eventually want to take a more consistent pass over this to deal with how we "materialize" values for setters.

- Update `RWStructuredBuffer` to have a `ref` accessor, and then fix up the IR tests to handle the new opcode that I added for it.
  - Note: I didn't handle this as an intrinsic simply because the `tests/ir/*` tests aren't really set up to handle builtins with ugly mangled names.

* Fixup: type error in VM for buffer element ref

I was using the result type of the op as the element type for computing the element address, but the result type is a pointer to the real element type.
This caused test failures on 64-bit platforms, where the stride of the buffer in the `ir/factorial` test needs to be 4.

The fix is to assume the result type is a pointer, and extract the pointed-to type out of that.

Author: Tim Foley

source/slang/check.cpp

@@ -3575,6 +3575,32 @@ namespace Slang
 
             decl->SetCheckState(DeclCheckState::CheckedHeader);
 
+            // If we have a subscript declaration with no accessor declarations,
+            // then we should create a single `GetterDecl` to represent
+            // the implicit meaning of their declaration, so:
+            //
+            //      subscript(uint index) -> T;
+            //
+            // becomes:
+            //
+            //      subscript(uint index) -> T { get; }
+            //
+
+            bool anyAccessors = false;
+            for(auto accessorDecl : decl->getMembersOfType<AccessorDecl>())
+            {
+                anyAccessors = true;
+            }
+
+            if(!anyAccessors)
+            {
+                RefPtr<GetterDecl> getterDecl = new GetterDecl();
+                getterDecl->loc = decl->loc;
+
+                getterDecl->ParentDecl = decl;
+                decl->Members.Add(getterDecl);
+            }
+
             for(auto mm : decl->Members)
             {
                 checkDecl(mm);
@@ -4662,6 +4688,10 @@ namespace Slang
                             {
                                 callExpr->type.IsLeftValue = true;
                             }
+                            for(auto refAccessor : subscriptDeclRef.getDecl()->getMembersOfType<RefAccessorDecl>())
+                            {
+                                callExpr->type.IsLeftValue = true;
+                            }
                         }
 
                         // TODO: there may be other cases that confer l-value-ness

source/slang/decl-defs.h

@@ -179,6 +179,7 @@ SIMPLE_SYNTAX_CLASS(AccessorDecl, FunctionDeclBase)
 
 SIMPLE_SYNTAX_CLASS(GetterDecl, AccessorDecl)
 SIMPLE_SYNTAX_CLASS(SetterDecl, AccessorDecl)
+SIMPLE_SYNTAX_CLASS(RefAccessorDecl, AccessorDecl)
 
 SIMPLE_SYNTAX_CLASS(FuncDecl, FunctionDeclBase)
 

source/slang/emit.cpp

@@ -4875,6 +4875,7 @@ emitDeclImpl(decl, nullptr);
         case kIROp_FieldAddress:
         case kIROp_getElementPtr:
         case kIROp_specialize:
+        case kIROp_BufferElementRef:
             return true;
         }
 
@@ -5516,6 +5517,7 @@ emitDeclImpl(decl, nullptr);
             break;
 
         case kIROp_BufferLoad:
+        case kIROp_BufferElementRef:
             emitIROperand(ctx, inst->getArg(0));
             emit("[");
             emitIROperand(ctx, inst->getArg(1));

source/slang/hlsl.meta.slang

@@ -44,8 +44,7 @@ struct StructuredBuffer
     T Load(int location);
     T Load(int location, out uint status);
 
-    __intrinsic_op(bufferLoad)
-    __subscript(uint index) -> T;
+    __subscript(uint index) -> T { __intrinsic_op(bufferLoad) get; };
 };
 
 __generic<T> __magic_type(HLSLConsumeStructuredBufferType) struct ConsumeStructuredBuffer
@@ -201,11 +200,8 @@ struct RWStructuredBuffer
 
 	__subscript(uint index) -> T
 	{
-		__intrinsic_op(bufferLoad)
-		get;
-
-		__intrinsic_op(bufferStore)
-		set;
+        __intrinsic_op(bufferElementRef)
+        ref;
 	}
 };
 

source/slang/hlsl.meta.slang.h

@@ -45,8 +45,7 @@ sb << "\n";
 sb << "    T Load(int location);\n";
 sb << "    T Load(int location, out uint status);\n";
 sb << "\n";
-sb << "    __intrinsic_op(bufferLoad)\n";
-sb << "    __subscript(uint index) -> T;\n";
+sb << "    __subscript(uint index) -> T { __intrinsic_op(bufferLoad) get; };\n";
 sb << "};\n";
 sb << "\n";
 sb << "__generic<T> __magic_type(HLSLConsumeStructuredBufferType) struct ConsumeStructuredBuffer\n";
@@ -203,11 +202,8 @@ sb << "    T Load(int location, out uint status);\n";
 sb << "\n";
 sb << "\t__subscript(uint index) -> T\n";
 sb << "\t{\n";
-sb << "\t\t__intrinsic_op(bufferLoad)\n";
-sb << "\t\tget;\n";
-sb << "\n";
-sb << "\t\t__intrinsic_op(bufferStore)\n";
-sb << "\t\tset;\n";
+sb << "        __intrinsic_op(bufferElementRef)\n";
+sb << "        ref;\n";
 sb << "\t}\n";
 sb << "};\n";
 sb << "\n";

source/slang/ir-inst-defs.h

@@ -119,6 +119,7 @@ INST(Store, store, 2, 0)
 
 INST(BufferLoad, bufferLoad, 2, 0)
 INST(BufferStore, bufferStore, 3, 0)
+INST(BufferElementRef, bufferElementRef, 2, 0)
 
 INST(FieldExtract, get_field, 2, 0)
 INST(FieldAddress, get_field_addr, 2, 0)