commit cc5fca057ec61748e5fbde429adee155c7392510
author Spencer Fricke <spencerfricke@gmail.com> Wed Jul 27 04:14:21 2022 +0900
committer GitHub <noreply@github.com> Tue Jul 26 15:14:21 2022 -0400
tree dac6b4cfbe671a65fee85c7655f03c6c47e28b2f
parent 388ce0ee64b393c02843fb7469647069c89ab5da

spirv-val: Fix Vulkan memory scope (#4869)

source/val/validate_scopes.cpp

diff --git a/source/val/validate_scopes.cpp b/source/val/validate_scopes.cpp
index 1c5f70a..887e8d1 100644
--- a/source/val/validate_scopes.cpp
+++ b/source/val/validate_scopes.cpp
@@ -220,30 +220,23 @@
 
   // Vulkan Specific rules
   if (spvIsVulkanEnv(_.context()->target_env)) {
-    if (value == SpvScopeCrossDevice) {
-      return _.diag(SPV_ERROR_INVALID_DATA, inst)
-             << _.VkErrorID(4638) << spvOpcodeString(opcode)
-             << ": in Vulkan environment, Memory Scope cannot be CrossDevice";
-    }
-    // Vulkan 1.0 specific rules
-    if (_.context()->target_env == SPV_ENV_VULKAN_1_0 &&
-        value != SpvScopeDevice && value != SpvScopeWorkgroup &&
-        value != SpvScopeInvocation) {
-      return _.diag(SPV_ERROR_INVALID_DATA, inst)
-             << _.VkErrorID(4638) << spvOpcodeString(opcode)
-             << ": in Vulkan 1.0 environment Memory Scope is limited to "
-             << "Device, Workgroup and Invocation";
-    }
-    // Vulkan 1.1 specific rules
-    if ((_.context()->target_env == SPV_ENV_VULKAN_1_1 ||
-         _.context()->target_env == SPV_ENV_VULKAN_1_2) &&
-        value != SpvScopeDevice && value != SpvScopeWorkgroup &&
+    if (value != SpvScopeDevice && value != SpvScopeWorkgroup &&
         value != SpvScopeSubgroup && value != SpvScopeInvocation &&
-        value != SpvScopeShaderCallKHR) {
+        value != SpvScopeShaderCallKHR && value != SpvScopeQueueFamily) {
       return _.diag(SPV_ERROR_INVALID_DATA, inst)
              << _.VkErrorID(4638) << spvOpcodeString(opcode)
-             << ": in Vulkan 1.1 and 1.2 environment Memory Scope is limited "
-             << "to Device, Workgroup, Invocation, and ShaderCall";
+             << ": in Vulkan environment Memory Scope is limited to Device, "
+                "QueueFamily, Workgroup, ShaderCallKHR, Subgroup, or "
+                "Invocation";
+    } else if (_.context()->target_env == SPV_ENV_VULKAN_1_0 &&
+               value == SpvScopeSubgroup &&
+               !_.HasCapability(SpvCapabilitySubgroupBallotKHR) &&
+               !_.HasCapability(SpvCapabilitySubgroupVoteKHR)) {
+      return _.diag(SPV_ERROR_INVALID_DATA, inst)
+             << spvOpcodeString(opcode)
+             << ": in Vulkan 1.0 environment Memory Scope is can not be "
+                "Subgroup without SubgroupBallotKHR or SubgroupVoteKHR "
+                "declared";
     }
 
     if (value == SpvScopeShaderCallKHR) {

test/val/val_barriers_test.cpp

diff --git a/test/val/val_barriers_test.cpp b/test/val/val_barriers_test.cpp
index 1178ca0..f27e467 100644
--- a/test/val/val_barriers_test.cpp
+++ b/test/val/val_barriers_test.cpp
@@ -359,12 +359,25 @@
 
   CompileSuccessfully(GenerateShaderCode(body), SPV_ENV_VULKAN_1_0);
   ASSERT_EQ(SPV_ERROR_INVALID_DATA, ValidateInstructions(SPV_ENV_VULKAN_1_0));
-  EXPECT_THAT(getDiagnosticString(),
-              AnyVUID("VUID-StandaloneSpirv-None-04638"));
   EXPECT_THAT(
       getDiagnosticString(),
-      HasSubstr("ControlBarrier: in Vulkan 1.0 environment Memory Scope is "
-                "limited to Device, Workgroup and Invocation"));
+      HasSubstr(
+          "ControlBarrier: in Vulkan 1.0 environment Memory Scope is can not "
+          "be Subgroup without SubgroupBallotKHR or SubgroupVoteKHR declared"));
+}
+
+TEST_F(ValidateBarriers, OpControlBarrierVulkanMemoryScopeSubgroupVoteKHR) {
+  const std::string capabilities = R"(
+OpCapability SubgroupVoteKHR
+OpExtension "SPV_KHR_subgroup_vote"
+)";
+  const std::string body = R"(
+OpControlBarrier %subgroup %subgroup %none
+)";
+
+  CompileSuccessfully(GenerateShaderCode(body, capabilities),
+                      SPV_ENV_VULKAN_1_0);
+  ASSERT_EQ(SPV_SUCCESS, ValidateInstructions(SPV_ENV_VULKAN_1_0));
 }
 
 TEST_F(ValidateBarriers, OpControlBarrierVulkan1p1MemoryScopeSubgroup) {
@@ -386,8 +399,9 @@
   EXPECT_THAT(getDiagnosticString(),
               AnyVUID("VUID-StandaloneSpirv-None-04638"));
   EXPECT_THAT(getDiagnosticString(),
-              HasSubstr("ControlBarrier: in Vulkan environment, Memory Scope "
-                        "cannot be CrossDevice"));
+              HasSubstr("ControlBarrier: in Vulkan environment Memory Scope is "
+                        "limited to Device, QueueFamily, Workgroup, "
+                        "ShaderCallKHR, Subgroup, or Invocation"));
 }
 
 TEST_F(ValidateBarriers,
@@ -751,12 +765,11 @@
 
   CompileSuccessfully(GenerateShaderCode(body), SPV_ENV_VULKAN_1_0);
   ASSERT_EQ(SPV_ERROR_INVALID_DATA, ValidateInstructions(SPV_ENV_VULKAN_1_0));
-  EXPECT_THAT(getDiagnosticString(),
-              AnyVUID("VUID-StandaloneSpirv-None-04638"));
   EXPECT_THAT(
       getDiagnosticString(),
-      HasSubstr("MemoryBarrier: in Vulkan 1.0 environment Memory Scope is "
-                "limited to Device, Workgroup and Invocation"));
+      HasSubstr(
+          "MemoryBarrier: in Vulkan 1.0 environment Memory Scope is can not be "
+          "Subgroup without SubgroupBallotKHR or SubgroupVoteKHR declared"));
 }
 
 TEST_F(ValidateBarriers, OpMemoryBarrierVulkan1p1MemoryScopeSubgroup) {