Merge pull request #736 from steve-lunarg/structbuffer-params

SPIRV/GlslangToSpv.cpp

@@ -2740,7 +2740,8 @@ void TGlslangToSpvTraverser::makeFunctions(const glslang::TIntermSequence& glslF
         for (int p = 0; p < (int)parameters.size(); ++p) {
             const glslang::TType& paramType = parameters[p]->getAsTyped()->getType();
             spv::Id typeId = convertGlslangToSpvType(paramType);
-            if (paramType.containsOpaque())
+            if (paramType.containsOpaque() ||
+                (paramType.getBasicType() == glslang::EbtBlock && paramType.getQualifier().storage == glslang::EvqBuffer))
                 typeId = builder.makePointer(TranslateStorageClass(paramType), typeId);
             else if (paramType.getQualifier().storage != glslang::EvqConstReadOnly)
                 typeId = builder.makePointer(spv::StorageClassFunction, typeId);
@@ -3220,7 +3221,8 @@ spv::Id TGlslangToSpvTraverser::handleUserFunctionCall(const glslang::TIntermAgg
     for (int a = 0; a < (int)glslangArgs.size(); ++a) {
         const glslang::TType& paramType = glslangArgs[a]->getAsTyped()->getType();
         spv::Id arg;
-        if (paramType.containsOpaque()) {
+        if (paramType.containsOpaque() ||
+            (paramType.getBasicType() == glslang::EbtBlock && qualifiers[a] == glslang::EvqBuffer)) {
             builder.setAccessChain(lValues[lValueCount]);
             arg = builder.accessChainGetLValue();
             ++lValueCount;

Test/baseResults/spv.ssbo.autoassign.frag.out

@@ -16,15 +16,15 @@ spv.ssbo.autoassign.frag
                               MemberName 14(BufType) 0  "va"
                               MemberName 14(BufType) 1  "vb"
                               Name 16  "SB0"
-                              MemberName 16(SB0) 0  "SB0"
-                              Name 18  ""
+                              MemberName 16(SB0) 0  "@data"
+                              Name 18  "SB0"
                               Name 26  "TestCB"
                               MemberName 26(TestCB) 0  "W"
                               MemberName 26(TestCB) 1  "H"
                               Name 28  ""
                               Name 55  "SB1"
-                              MemberName 55(SB1) 0  "SB1"
-                              Name 57  ""
+                              MemberName 55(SB1) 0  "@data"
+                              Name 57  "SB1"
                               Name 86  "pos"
                               Name 88  "pos"
                               Name 91  "@entryPointOutput"
@@ -37,8 +37,8 @@ spv.ssbo.autoassign.frag
                               MemberDecorate 16(SB0) 0 NonWritable
                               MemberDecorate 16(SB0) 0 Offset 0
                               Decorate 16(SB0) BufferBlock
-                              Decorate 18 DescriptorSet 0
-                              Decorate 18 Binding 30
+                              Decorate 18(SB0) DescriptorSet 0
+                              Decorate 18(SB0) Binding 30
                               MemberDecorate 26(TestCB) 0 Offset 0
                               MemberDecorate 26(TestCB) 1 Offset 4
                               Decorate 26(TestCB) Block
@@ -47,8 +47,8 @@ spv.ssbo.autoassign.frag
                               Decorate 54 ArrayStride 32
                               MemberDecorate 55(SB1) 0 Offset 0
                               Decorate 55(SB1) BufferBlock
-                              Decorate 57 DescriptorSet 0
-                              Decorate 57 Binding 31
+                              Decorate 57(SB1) DescriptorSet 0
+                              Decorate 57(SB1) Binding 31
                               Decorate 88(pos) Location 0
                               Decorate 91(@entryPointOutput) Location 0
                2:             TypeVoid
@@ -61,7 +61,7 @@ spv.ssbo.autoassign.frag
               15:             TypeRuntimeArray 14(BufType)
          16(SB0):             TypeStruct 15
               17:             TypePointer Uniform 16(SB0)
-              18:     17(ptr) Variable Uniform
+         18(SB0):     17(ptr) Variable Uniform
               19:             TypeInt 32 1
               20:     19(int) Constant 0
               21:             TypeInt 32 0
@@ -77,7 +77,7 @@ spv.ssbo.autoassign.frag
               54:             TypeRuntimeArray 14(BufType)
          55(SB1):             TypeStruct 54
               56:             TypePointer Uniform 55(SB1)
-              57:     56(ptr) Variable Uniform
+         57(SB1):     56(ptr) Variable Uniform
               87:             TypePointer Input 7(fvec4)
          88(pos):     87(ptr) Variable Input
               90:             TypePointer Output 7(fvec4)
@@ -107,7 +107,7 @@ spv.ssbo.autoassign.frag
               35:     23(ptr) AccessChain 10(pos) 34
               36:    6(float) Load 35
               37:    6(float) FAdd 33 36
-              39:     38(ptr) AccessChain 18 20 37 20
+              39:     38(ptr) AccessChain 18(SB0) 20 37 20
               40:    7(fvec4) Load 39
               41:     23(ptr) AccessChain 10(pos) 22
               42:    6(float) Load 41
@@ -118,7 +118,7 @@ spv.ssbo.autoassign.frag
               47:     23(ptr) AccessChain 10(pos) 34
               48:    6(float) Load 47
               49:    6(float) FAdd 46 48
-              51:     38(ptr) AccessChain 18 20 49 50
+              51:     38(ptr) AccessChain 18(SB0) 20 49 50
               52:    7(fvec4) Load 51
               53:    7(fvec4) FAdd 40 52
                               Store 13(vTmp) 53
@@ -131,7 +131,7 @@ spv.ssbo.autoassign.frag
               64:     23(ptr) AccessChain 10(pos) 34
               65:    6(float) Load 64
               66:    6(float) FAdd 63 65
-              67:     38(ptr) AccessChain 57 20 66 20
+              67:     38(ptr) AccessChain 57(SB1) 20 66 20
               68:    7(fvec4) Load 67
               69:     23(ptr) AccessChain 10(pos) 22
               70:    6(float) Load 69
@@ -142,7 +142,7 @@ spv.ssbo.autoassign.frag
               75:     23(ptr) AccessChain 10(pos) 34
               76:    6(float) Load 75
               77:    6(float) FAdd 74 76
-              78:     38(ptr) AccessChain 57 20 77 50
+              78:     38(ptr) AccessChain 57(SB1) 20 77 50
               79:    7(fvec4) Load 78
               80:    7(fvec4) FAdd 68 79
               81:    7(fvec4) Load 13(vTmp)

Test/hlsl.structbuffer.fn.frag

+
+StructuredBuffer<uint4>  sbuf : register(t10);
+RWStructuredBuffer<uint4> sbuf2;
+
+// Not shared, because of type difference.
+StructuredBuffer<uint3>  sbuf3 : register(t12);
+
+uint4 get(in StructuredBuffer<uint4> sb, uint bufferOffset)
+{
+    return sb[bufferOffset];
+}
+
+void set(in RWStructuredBuffer<uint4> sb, uint bufferOffset, uint4 data)
+{
+    sb[bufferOffset] = data;
+}
+
+float4 main(uint pos : FOO) : SV_Target0
+{
+    set(sbuf2, 2, get(sbuf, 3));
+
+    return 0;
+}

glslang/MachineIndependent/SymbolTable.cpp

@@ -103,7 +103,11 @@ void TType::buildMangledName(TString& mangledName)
             mangledName += "M";
         break;
     case EbtStruct:
-        mangledName += "struct-";
+    case EbtBlock:
+        if (basicType == EbtStruct)
+            mangledName += "struct-";
+        else
+            mangledName += "block-";
         if (typeName)
             mangledName += *typeName;
         for (unsigned int i = 0; i < structure->size(); ++i) {

gtests/Hlsl.FromFile.cpp

@@ -226,6 +226,7 @@ INSTANTIATE_TEST_CASE_P(
         {"hlsl.structbuffer.atomics.frag", "main"},
         {"hlsl.structbuffer.byte.frag", "main"},
         {"hlsl.structbuffer.coherent.frag", "main"},
+        {"hlsl.structbuffer.fn.frag", "main"},
         {"hlsl.structbuffer.rw.frag", "main"},
         {"hlsl.structbuffer.rwbyte.frag", "main"},
         {"hlsl.structin.vert", "main"},

hlsl/hlslGrammar.cpp

@@ -406,36 +406,13 @@ bool HlslGrammar::acceptDeclaration(TIntermNode*& node, TIntermNode*& node2)
                 }
             }
 
-            TString* blockName = idToken.string;
-
-            // For structbuffers, we couldn't create the block type while accepting the
-            // template type, because we need the identifier name.  Now that we have that,
-            // we can create the buffer type.
-            // TODO: how to determine this without looking for implicit array sizes?
-            if (variableType.getBasicType() == EbtBlock) {
-                const int memberCount = variableType.getStruct()->size();
-                assert(memberCount > 0);
-
-                TType* contentType = (*variableType.getStruct())[memberCount-1].type;
-                
-                // Set the field name and qualifier from the declaration, now that we know it.
-                if (contentType->isRuntimeSizedArray()) {
-                    contentType->getQualifier() = variableType.getQualifier();
-                    blockName     = nullptr;        // this will be an anonymous block...
-                    contentType->setFieldName(*idToken.string); // field name is declaration name
-                    variableType.setTypeName(*idToken.string);
-                }
-            }
-
-            // Hand off the actual declaration
-
             // TODO: things scoped within an annotation need their own name space;
             // TODO: strings are not yet handled.
             if (variableType.getBasicType() != EbtString && parseContext.getAnnotationNestingLevel() == 0) {
                 if (typedefDecl)
                     parseContext.declareTypedef(idToken.loc, *idToken.string, variableType);
                 else if (variableType.getBasicType() == EbtBlock)
-                    parseContext.declareBlock(idToken.loc, variableType, blockName);
+                    parseContext.declareBlock(idToken.loc, variableType, idToken.string);
                 else {
                     if (variableType.getQualifier().storage == EvqUniform && ! variableType.containsOpaque()) {
                         // this isn't really an individual variable, but a member of the $Global buffer
@@ -1888,18 +1865,26 @@ bool HlslGrammar::acceptStructBufferType(TType& type)
     TArraySizes unsizedArray;
     unsizedArray.addInnerSize(UnsizedArraySize);
     templateType->newArraySizes(unsizedArray);
-    templateType->getQualifier().storage = storage;
-    templateType->getQualifier().readonly = readonly;
+
+    // field name is canonical for all structbuffers
+    templateType->setFieldName("@data");
 
     // Create block type.  TODO: hidden internal uint member when needed
+
     TTypeList* blockStruct = new TTypeList;
     TTypeLoc  member = { templateType, token.loc };
     blockStruct->push_back(member);
 
+    // This is the type of the buffer block (SSBO)
     TType blockType(blockStruct, "", templateType->getQualifier());
 
-    // It's not until we see the name during declaration that we can set the
-    // field name.  That happens in HlslGrammar::acceptDeclaration.
+    blockType.getQualifier().storage = storage;
+    blockType.getQualifier().readonly = readonly;
+
+    // We may have created an equivalent type before, in which case we should use its
+    // deep structure.
+    parseContext.shareStructBufferType(blockType);
+
     type.shallowCopy(blockType);
 
     return true;

hlsl/hlslParseHelper.cpp

@@ -691,6 +691,22 @@ TIntermTyped* HlslParseContext::handleBracketOperator(const TSourceLoc& loc, TIn
         }
     }
 
+    // Handle operator[] on structured buffers: this indexes into the array element of the buffer.
+    // indexStructBufferContent returns nullptr if it isn't a structuredbuffer (SSBO).
+    TIntermTyped* sbArray = indexStructBufferContent(loc, base);
+    if (sbArray != nullptr) {
+        if (sbArray == nullptr)
+            return nullptr;
+
+        // Now we'll apply the [] index to that array
+        const TOperator idxOp = (index->getQualifier().storage == EvqConst) ? EOpIndexDirect : EOpIndexIndirect;
+
+        TIntermTyped* element = intermediate.addIndex(idxOp, sbArray, index, loc);
+        const TType derefType(sbArray->getType(), 0);
+        element->setType(derefType);
+        return element;
+    }
+
     return nullptr;
 }
 
@@ -866,9 +882,7 @@ TIntermTyped* HlslParseContext::handleDotDereference(const TSourceLoc& loc, TInt
             const int vecSize = sampler.isShadow() ? 1 : 4; // TODO: handle arbitrary sample return sizes
             return intermediate.addMethod(base, TType(sampler.type, EvqTemporary, vecSize), &field, loc);
         }
-    } else if (isStructBufferMethod(field) && 
-               base->getType().isRuntimeSizedArray() &&
-               (base->getQualifier().storage == EvqUniform || base->getQualifier().storage == EvqBuffer)) {
+    } else if (isStructBufferType(base->getType())) {
         TType retType(base->getType(), 0);
         return intermediate.addMethod(base, retType, &field, loc);
     } else if (field == "Append" ||
@@ -1919,9 +1933,11 @@ void HlslParseContext::remapNonEntryPointIO(TFunction& function)
     if (function.getType().getBasicType() != EbtVoid)
         clearUniformInputOutput(function.getWritableType().getQualifier());
 
-    // parameters
+    // parameters.
+    // References to structuredbuffer types are left unmodified
     for (int i = 0; i < function.getParamCount(); i++)
-        clearUniformInputOutput(function[i].type->getQualifier());
+        if (!isReference(*function[i].type))
+            clearUniformInputOutput(function[i].type->getQualifier());
 }
 
 // Handle function returns, including type conversions to the function return type
@@ -2284,12 +2300,16 @@ void HlslParseContext::decomposeStructBufferMethods(const TSourceLoc& loc, TInte
 
     const TOperator op  = node->getAsOperator()->getOp();
     TIntermAggregate* argAggregate = arguments ? arguments->getAsAggregate() : nullptr;
+    if (argAggregate == nullptr)
+        return;
 
-    TIntermTyped* argArray = argAggregate ? argAggregate->getSequence()[0]->getAsTyped() : nullptr;  // array
+    // Buffer is the object upon which method is called, so always arg 0
+    TIntermTyped* bufferObj = argAggregate->getSequence()[0]->getAsTyped();
 
-    // Bail out if not a block method
-    if (argArray == nullptr || !argArray->getType().isRuntimeSizedArray())
-        return;
+    // Index to obtain the runtime sized array out of the buffer.
+    TIntermTyped* argArray = indexStructBufferContent(loc, bufferObj);
+    if (argArray == nullptr)
+        return;  // It might not be a struct buffer method.
 
     switch (op) {
     case EOpMethodLoad:
@@ -3740,7 +3760,7 @@ TIntermTyped* HlslParseContext::handleFunctionCall(const TSourceLoc& loc, TFunct
         // the symbol table for an arbitrary type.  This is a temporary hack until that ability exists.
         // It will have false positives, since it doesn't check arg counts or types.
         if (arguments && arguments->getAsAggregate()) {
-            if (arguments->getAsAggregate()->getSequence()[0]->getAsTyped()->getType().isRuntimeSizedArray()) {
+            if (isStructBufferType(arguments->getAsAggregate()->getSequence()[0]->getAsTyped()->getType())) {
                 if (isStructBufferMethod(function->getName())) {
                     const TString mangle = function->getName() + "(";
                     TSymbol* symbol = symbolTable.find(mangle, &builtIn);
@@ -5130,6 +5150,101 @@ void HlslParseContext::redeclareBuiltinBlock(const TSourceLoc& loc, TTypeList& n
     trackLinkage(*block);
 }
 
+//
+// Generate index to the array element in a structure buffer (SSBO)
+//
+TIntermTyped* HlslParseContext::indexStructBufferContent(const TSourceLoc& loc, TIntermTyped* buffer) const
+{
+    // Bail out if not a struct buffer
+    if (buffer == nullptr || ! isStructBufferType(buffer->getType()))
+        return nullptr;
+
+    // Runtime sized array is always the last element.
+    const TTypeList* bufferStruct = buffer->getType().getStruct();
+    TIntermTyped* arrayPosition = intermediate.addConstantUnion(unsigned(bufferStruct->size()-1), loc);
+
+    TIntermTyped* argArray = intermediate.addIndex(EOpIndexDirectStruct, buffer, arrayPosition, loc);
+    argArray->setType(*(*bufferStruct)[bufferStruct->size()-1].type);
+
+    return argArray;
+}
+
+//
+// IFF type is a structuredbuffer/byteaddressbuffer type, return the content
+// (template) type.   E.g, StructuredBuffer<MyType> -> MyType.  Else return nullptr.
+//
+TType* HlslParseContext::getStructBufferContentType(const TType& type) const
+{
+    if (type.getBasicType() != EbtBlock)
+        return nullptr;
+
+    const int memberCount = type.getStruct()->size();
+    assert(memberCount > 0);
+
+    TType* contentType = (*type.getStruct())[memberCount-1].type;
+
+    return contentType->isRuntimeSizedArray() ? contentType : nullptr;
+}
+
+//
+// If an existing struct buffer has a sharable type, then share it.
+//
+void HlslParseContext::shareStructBufferType(TType& type)
+{
+    // PackOffset must be equivalent to share types on a per-member basis.
+    // Note: cannot use auto type due to recursion.  Thus, this is a std::function.
+    const std::function<bool(TType& lhs, TType& rhs)>
+    compareQualifiers = [&](TType& lhs, TType& rhs) -> bool {
+        if (lhs.getQualifier().layoutOffset != rhs.getQualifier().layoutOffset)
+            return false;
+
+        if (lhs.isStruct() != rhs.isStruct())
+            return false;
+
+        if (lhs.isStruct() && rhs.isStruct()) {
+            if (lhs.getStruct()->size() != rhs.getStruct()->size())
+                return false;
+
+            for (int i = 0; i < int(lhs.getStruct()->size()); ++i)
+                if (!compareQualifiers(*(*lhs.getStruct())[i].type, *(*rhs.getStruct())[i].type))
+                    return false;
+        }
+
+        return true;
+    };
+
+    // We need to compare certain qualifiers in addition to the type.
+    const auto typeEqual = [compareQualifiers](TType& lhs, TType& rhs) -> bool {
+        if (lhs.getQualifier().readonly != rhs.getQualifier().readonly)
+            return false;
+
+        // If both are structures, recursively look for packOffset equality
+        // as well as type equality.
+        return compareQualifiers(lhs, rhs) && lhs == rhs;
+    };
+
+    // TString typeName;
+    // type.appendMangledName(typeName);
+    // type.setTypeName(typeName);
+
+    // This is an exhaustive O(N) search, but real world shaders have
+    // only a small number of these.
+    for (int idx = 0; idx < int(structBufferTypes.size()); ++idx) {
+        // If the deep structure matches, modulo qualifiers, use it
+        if (typeEqual(*structBufferTypes[idx], type)) {
+            type.shallowCopy(*structBufferTypes[idx]);
+            return;
+        }
+    }
+
+    // Otherwise, remember it:
+    TType* typeCopy = new TType;
+    typeCopy->shallowCopy(type);
+    structBufferTypes.push_back(typeCopy);
+
+    // structBuffTypes.push_back(type.getWritableStruct());
+}
+
 void HlslParseContext::paramFix(TType& type)
 {
     switch (type.getQualifier().storage) {
@@ -5140,6 +5255,18 @@ void HlslParseContext::paramFix(TType& type)
     case EvqTemporary:
         type.getQualifier().storage = EvqIn;
         break;
+    case EvqBuffer:
+        {
+            // SSBO parameter.  These do not go through the declareBlock path since they are fn parameters.
+            correctUniform(type.getQualifier());
+            TQualifier bufferQualifier = globalBufferDefaults;
+            mergeObjectLayoutQualifiers(bufferQualifier, type.getQualifier(), true);
+            bufferQualifier.storage = type.getQualifier().storage;
+            bufferQualifier.readonly = type.getQualifier().readonly;
+            bufferQualifier.coherent = type.getQualifier().coherent;
+            type.getQualifier() = bufferQualifier;
+            break;
+        }
     default:
         break;
     }
@@ -6011,6 +6138,7 @@ TIntermNode* HlslParseContext::declareVariable(const TSourceLoc& loc, TString& i
             if (it != ioTypeMap.end())
                 type.setStruct(it->second.uniform);
         }
+
         break;
     default:
         break;
@@ -6648,8 +6776,6 @@ void HlslParseContext::declareBlock(const TSourceLoc& loc, TType& type, const TS
                 error(memberLoc, "member cannot contradict block (or what block inherited from global)", "xfb_buffer", "");
         }
 
-        if (memberQualifier.hasPacking())
-            error(memberLoc, "member of block cannot have a packing layout qualifier", typeList[member].type->getFieldName().c_str(), "");
         if (memberQualifier.hasLocation()) {
             switch (type.getQualifier().storage) {
             case EvqVaryingIn:
@@ -6661,10 +6787,6 @@ void HlslParseContext::declareBlock(const TSourceLoc& loc, TType& type, const TS
             }
         } else
             memberWithoutLocation = true;
-        if (memberQualifier.hasAlign()) {
-            if (defaultQualification.layoutPacking != ElpStd140 && defaultQualification.layoutPacking != ElpStd430)
-                error(memberLoc, "can only be used with std140 or std430 layout packing", "align", "");
-        }
 
         TQualifier newMemberQualification = defaultQualification;
         mergeQualifiers(newMemberQualification, memberQualifier);

hlsl/hlslParseHelper.h

@@ -180,6 +180,9 @@ public:
     void initFlattening() { flattenLevel.push_back(0); flattenOffset.push_back(0); }
     void finalizeFlattening() { flattenLevel.pop_back(); flattenOffset.pop_back(); }
 
+    // Share struct buffer deep types
+    void shareStructBufferType(TType&);
+
 protected:
     struct TFlattenData {
         TFlattenData() : nextBinding(TQualifier::layoutBindingEnd) { }
@@ -248,6 +251,14 @@ protected:
     bool isSamplerMethod(const TString& name) const;
     bool isStructBufferMethod(const TString& name) const;
 
+    TType* getStructBufferContentType(const TType& type) const;
+    bool isStructBufferType(const TType& type) const { return getStructBufferContentType(type) != nullptr; }
+    TIntermTyped* indexStructBufferContent(const TSourceLoc& loc, TIntermTyped* buffer) const;
+
+    // Return true if this type is a reference.  This is not currently a type method in case that's
+    // a language specific answer.
+    bool isReference(const TType& type) const { return isStructBufferType(type); }
+
     // Pass through to base class after remembering builtin mappings.
     using TParseContextBase::trackLinkage;
     void trackLinkage(TSymbol& variable) override;
@@ -330,6 +341,9 @@ protected:
     // Structure splitting data:
     TMap<int, TVariable*>              splitIoVars;  // variables with the builtin interstage IO removed, indexed by unique ID.
 
+    // Structuredbuffer shared types.  Typically there are only a few.
+    TVector<TType*> structBufferTypes;
+
     // The builtin interstage IO map considers e.g, EvqPosition on input and output separately, so that we
     // can build the linkage correctly if position appears on both sides.  Otherwise, multiple positions
     // are considered identical.