Support SPV_KHR_storage_buffer_storage_class

Includes interaction with SPV_KHR_variable_pointers TODO: Update capabilitly dependency tests for new capabilities.
alan-baker · Apr 7, 2017 · bf68c81 · bf68c81
1 parent 23af06c
commit bf68c81
Show file tree

Hide file tree

Showing 8 changed files with 43 additions and 23 deletions.
diff --git a/source/val/validation_state.cpp b/source/val/validation_state.cpp
@@ -303,10 +303,10 @@ void ValidationState_t::RegisterCapability(SpvCapability cap) {
       break;
     case SpvCapabilityVariablePointers:
       features_.variable_pointers = true;
-      features_.variable_pointers_uniform_buffer_block = true;
+      features_.variable_pointers_storage_buffer = true;
       break;
-    case SpvCapabilityVariablePointersUniformBufferBlock:
-      features_.variable_pointers_uniform_buffer_block = true;
+    case SpvCapabilityVariablePointersStorageBuffer:
+      features_.variable_pointers_storage_buffer = true;
       break;
     default:
       break;

diff --git a/source/val/validation_state.h b/source/val/validation_state.h
@@ -65,9 +65,9 @@ class ValidationState_t {
 
     // Allow functionalities enabled by VariablePointers capability.
     bool variable_pointers = false;
-    // Allow functionalities enabled by VariablePointersUniformBufferBlock
+    // Allow functionalities enabled by VariablePointersStorageBuffer
     // capability.
-    bool variable_pointers_uniform_buffer_block = false;
+    bool variable_pointers_storage_buffer = false;
   };
 
   ValidationState_t(const spv_const_context context,

diff --git a/source/validate_id.cpp b/source/validate_id.cpp
@@ -1118,7 +1118,7 @@ bool idUsage::isValid<SpvOpLoad>(const spv_instruction_t* inst,
   }
   const bool uses_variable_pointer =
       module_.features().variable_pointers ||
-      module_.features().variable_pointers_uniform_buffer_block;
+      module_.features().variable_pointers_storage_buffer;
   auto pointerIndex = 3;
   auto pointer = module_.FindDef(inst->words[pointerIndex]);
   if (!pointer ||
@@ -1154,7 +1154,7 @@ bool idUsage::isValid<SpvOpStore>(const spv_instruction_t* inst,
                                   const spv_opcode_desc) {
   const bool uses_variable_pointer =
       module_.features().variable_pointers ||
-      module_.features().variable_pointers_uniform_buffer_block;
+      module_.features().variable_pointers_storage_buffer;
   const auto pointerIndex = 1;
   auto pointer = module_.FindDef(inst->words[pointerIndex]);
   if (!pointer ||
@@ -2420,7 +2420,7 @@ bool idUsage::isValid<SpvOpReturnValue>(const spv_instruction_t* inst,
   }
   const bool uses_variable_pointer =
       module_.features().variable_pointers ||
-      module_.features().variable_pointers_uniform_buffer_block;
+      module_.features().variable_pointers_storage_buffer;
   if (addressingModel == SpvAddressingModelLogical &&
       SpvOpTypePointer == valueType->opcode() && !uses_variable_pointer) {
     DIAG(valueIndex)

diff --git a/test/enum_string_mapping_test.cpp b/test/enum_string_mapping_test.cpp
@@ -221,9 +221,13 @@ INSTANTIATE_TEST_CASE_P(AllCapabilities, CapabilityTest,
       {SpvCapabilitySubgroupVoteKHR,
        "SubgroupVoteKHR"},
       {SpvCapabilityStorageUniformBufferBlock16,
-       "StorageUniformBufferBlock16"},
+       "StorageBuffer16BitAccess"},
+      {SpvCapabilityStorageBuffer16BitAccess,
+       "StorageBuffer16BitAccess"},
       {SpvCapabilityStorageUniform16,
-       "StorageUniform16"},
+       "UniformAndStorageBuffer16BitAccess"},
+      {SpvCapabilityUniformAndStorageBuffer16BitAccess,
+       "UniformAndStorageBuffer16BitAccess"},
       {SpvCapabilityStoragePushConstant16,
        "StoragePushConstant16"},
       {SpvCapabilityStorageInputOutput16,

diff --git a/test/text_to_binary.extension_test.cpp b/test/text_to_binary.extension_test.cpp
@@ -197,10 +197,17 @@ INSTANTIATE_TEST_CASE_P(
     Combine(Values(SPV_ENV_UNIVERSAL_1_0, SPV_ENV_UNIVERSAL_1_1,
                    SPV_ENV_VULKAN_1_0),
             ValuesIn(std::vector<AssemblyCase>{
-                {"OpCapability StorageUniformBufferBlock16\n",
+                {"OpCapability StorageBuffer16BitAccess\n",
                  MakeInstruction(SpvOpCapability,
                                  {SpvCapabilityStorageUniformBufferBlock16})},
-                {"OpCapability StorageUniform16\n",
+                {"OpCapability StorageBuffer16BitAccess\n",
+                 MakeInstruction(SpvOpCapability,
+                                 {SpvCapabilityStorageBuffer16BitAccess})},
+                {"OpCapability UniformAndStorageBuffer16BitAccess\n",
+                 MakeInstruction(
+                     SpvOpCapability,
+                     {SpvCapabilityUniformAndStorageBuffer16BitAccess})},
+                {"OpCapability UniformAndStorageBuffer16BitAccess\n",
                  MakeInstruction(SpvOpCapability,
                                  {SpvCapabilityStorageUniform16})},
                 {"OpCapability StoragePushConstant16\n",
@@ -282,10 +289,10 @@ INSTANTIATE_TEST_CASE_P(
                 {"OpCapability VariablePointers\n",
                  MakeInstruction(SpvOpCapability,
                                  {SpvCapabilityVariablePointers})},
-                {"OpCapability VariablePointersUniformBufferBlock\n",
+                {"OpCapability VariablePointersStorageBuffer\n",
                  MakeInstruction(
                      SpvOpCapability,
-                     {SpvCapabilityVariablePointersUniformBufferBlock})},
+                     {SpvCapabilityVariablePointersStorageBuffer})},
             })), );
 
 }  // anonymous namespace
diff --git a/test/text_to_binary.type_declaration_test.cpp b/test/text_to_binary.type_declaration_test.cpp
@@ -213,6 +213,7 @@ TEST_F(OpTypeForwardPointerTest, ValidStorageClass) {
   CASE(PushConstant);
   CASE(AtomicCounter);
   CASE(Image);
+  CASE(StorageBuffer);
 }
 
 #undef CASE

diff --git a/test/val/val_id_test.cpp b/test/val/val_id_test.cpp
@@ -1806,7 +1806,7 @@ TEST_F(ValidateIdWithMessage, OpLoadGood) {
   EXPECT_EQ(SPV_SUCCESS, ValidateInstructions());
 }
 
-// TODO: Add tests that exercise VariablePointersUniformBufferBlock instead of
+// TODO: Add tests that exercise VariablePointersStorageBuffer instead of
 // VariablePointers.
 void createVariablePointerSpirvProgram(std::ostringstream* spirv,
                                        std::string result_strategy,

diff --git a/utils/generate_grammar_tables.py b/utils/generate_grammar_tables.py
@@ -364,11 +364,13 @@ def get_extension_list(operands):
     return sorted(set(extensions))
 
 
-def get_capability_list(operands):
-    """Returns capabilities as a list of strings in the order of appearance."""
+def get_capabilities(operands):
+    """Returns capabilities as a list of JSON objects, in order of
+    appearance.
+    """
     enumerants = sum([item.get('enumerants', []) for item in operands
                       if item.get('kind') in ['Capability']], [])
-    return [item.get('enumerant') for item in enumerants]
+    return enumerants
 
 
 def generate_extension_enum(operands):
@@ -399,7 +401,8 @@ def generate_string_to_extension_table(operands):
 
 def generate_capability_to_string_table(operands):
     """Returns capability to string mapping table."""
-    capabilities = get_capability_list(operands)
+    capabilities = [item.get('enumerant')
+                    for item in get_capabilities(operands)]
     entry_template = '  {{SpvCapability{capability},\n   "{capability}"}}'
     table_entries = [entry_template.format(capability=capability)
                      for capability in capabilities]
@@ -434,14 +437,19 @@ def generate_string_to_extension_mapping(operands):
 
 
 def generate_capability_to_string_mapping(operands):
-    """Returns mapping function from capabilities to corresponding strings."""
-    capabilities = get_capability_list(operands)
+    """Returns mapping function from capabilities to corresponding strings.
+    We take care to avoid emitting duplicate values.
+    """
     function = 'std::string CapabilityToString(SpvCapability capability) {\n'
     function += '  switch (capability) {\n'
     template = '    case SpvCapability{capability}:\n' \
         '      return "{capability}";\n'
-    function += ''.join([template.format(capability=capability)
-                         for capability in capabilities])
+    emitted = set() # The values of capabilities we already have emitted
+    for capability in get_capabilities(operands):
+        value = capability.get('value')
+        if value not in emitted:
+            emitted.add(value)
+            function += template.format(capability=capability.get('enumerant'))
     function += '    case SpvCapabilityMax:\n' \
         '      assert(0 && "Attempting to convert SpvCapabilityMax to string");\n' \
         '      return "";\n'