Support SPV_KHR_8bit_storage

- Add asm/dis test for SPV_KHR_8bit_storage - validator: SPV_KHR_8bit_storage capabilities enable declaration of 8bit int TODO: - validator: ban arithmetic on 8bit unless Int8 is enabled Covered by KhronosGroup#1595
EpicGames · Jun 20, 2018 · a9d3268 · a9d3268
1 parent 8d65c89
commit a9d3268
Show file tree

Hide file tree

Showing 6 changed files with 60 additions and 2 deletions.
diff --git a/source/val/validation_state.cpp b/source/val/validation_state.cpp
@@ -316,6 +316,12 @@ void ValidationState_t::RegisterCapability(SpvCapability cap) {
     case SpvCapabilityKernel:
       features_.group_ops_reduce_and_scans = true;
       break;
+    case SpvCapabilityInt8:
+    case SpvCapabilityStorageBuffer8BitAccess:
+    case SpvCapabilityUniformAndStorageBuffer8BitAccess:
+    case SpvCapabilityStoragePushConstant8:
+      features_.declare_int8_type = true;
+      break;
     case SpvCapabilityInt16:
       features_.declare_int16_type = true;
       break;

diff --git a/source/val/validation_state.h b/source/val/validation_state.h
@@ -74,6 +74,9 @@ class ValidationState_t {
 
     // Permit group oerations Reduce, InclusiveScan, ExclusiveScan
     bool group_ops_reduce_and_scans = false;
+
+    // Allow OpTypeInt with 8 bit width?
+    bool declare_int8_type = false;
   };
 
   ValidationState_t(const spv_const_context context,

diff --git a/source/validate_datarules.cpp b/source/validate_datarules.cpp
@@ -101,11 +101,12 @@ spv_result_t ValidateIntSize(ValidationState_t& _,
     return SPV_SUCCESS;
   }
   if (num_bits == 8) {
-    if (_.HasCapability(SpvCapabilityInt8)) {
+    if (_.features().declare_int8_type) {
       return SPV_SUCCESS;
     }
     return _.diag(SPV_ERROR_INVALID_DATA)
-           << "Using an 8-bit integer type requires the Int8 capability.";
+           << "Using an 8-bit integer type requires the Int8 capability,"
+              " or an extension that explicitly enables 16-bit integers.";
   }
   if (num_bits == 16) {
     if (_.features().declare_int16_type) {

diff --git a/test/enum_string_mapping_test.cpp b/test/enum_string_mapping_test.cpp
@@ -87,6 +87,7 @@ INSTANTIATE_TEST_CASE_P(
         {Extension::kSPV_GOOGLE_decorate_string, "SPV_GOOGLE_decorate_string"},
         {Extension::kSPV_GOOGLE_hlsl_functionality1,
          "SPV_GOOGLE_hlsl_functionality1"},
+        {Extension::kSPV_KHR_8bit_storage, "SPV_KHR_8bit_storage"},
     })));
 
 INSTANTIATE_TEST_CASE_P(UnknownExtensions, UnknownExtensionTest,

diff --git a/test/text_to_binary.extension_test.cpp b/test/text_to_binary.extension_test.cpp
@@ -316,6 +316,26 @@ INSTANTIATE_TEST_CASE_P(
                                                  SpvBuiltInDeviceIndex})},
             })), );
 
+// SPV_KHR_8bit_storage
+
+INSTANTIATE_TEST_CASE_P(
+    SPV_KHR_8bit_storage, ExtensionRoundTripTest,
+    // We'll get coverage over operand tables by trying the universal
+    // environments, and at least one specific environment.
+    Combine(
+        ValuesIn(CommonVulkanEnvs()),
+        ValuesIn(std::vector<AssemblyCase>{
+            {"OpCapability StorageBuffer8BitAccess\n",
+             MakeInstruction(SpvOpCapability,
+                             {SpvCapabilityStorageBuffer8BitAccess})},
+            {"OpCapability UniformAndStorageBuffer8BitAccess\n",
+             MakeInstruction(SpvOpCapability,
+                             {SpvCapabilityUniformAndStorageBuffer8BitAccess})},
+            {"OpCapability StoragePushConstant8\n",
+             MakeInstruction(SpvOpCapability,
+                             {SpvCapabilityStoragePushConstant8})},
+        })), );
+
 // SPV_KHR_multiview
 
 INSTANTIATE_TEST_CASE_P(

diff --git a/test/val/val_data_test.cpp b/test/val/val_data_test.cpp
@@ -224,6 +224,33 @@ TEST_F(ValidateData, int8_bad) {
   EXPECT_THAT(getDiagnosticString(), HasSubstr(missing_int8_cap_error));
 }
 
+TEST_F(ValidateData, int8_with_storage_buffer_8bit_access_good) {
+  string str = HeaderWith(
+                   "StorageBuffer8BitAccess "
+                   "OpExtension \"SPV_KHR_8bit_storage\"") +
+               " %2 = OpTypeInt 8 0";
+  CompileSuccessfully(str.c_str());
+  EXPECT_EQ(SPV_SUCCESS, ValidateInstructions()) << getDiagnosticString();
+}
+
+TEST_F(ValidateData, int8_with_uniform_and_storage_buffer_8bit_access_good) {
+  string str = HeaderWith(
+                   "UniformAndStorageBuffer8BitAccess "
+                   "OpExtension \"SPV_KHR_8bit_storage\"") +
+               " %2 = OpTypeInt 8 0";
+  CompileSuccessfully(str.c_str());
+  EXPECT_EQ(SPV_SUCCESS, ValidateInstructions()) << getDiagnosticString();
+}
+
+TEST_F(ValidateData, int8_with_storage_push_constant_8_good) {
+  string str = HeaderWith(
+                   "StoragePushConstant8 "
+                   "OpExtension \"SPV_KHR_8bit_storage\"") +
+               " %2 = OpTypeInt 8 0";
+  CompileSuccessfully(str.c_str());
+  EXPECT_EQ(SPV_SUCCESS, ValidateInstructions()) << getDiagnosticString();
+}
+
 TEST_F(ValidateData, int16_good) {
   string str = header_with_int16 + "%2 = OpTypeInt 16 1";
   CompileSuccessfully(str.c_str());