Update WebGPU validation rules of OpAtomic*s (#2777)
Fixes #2723
diff --git a/source/val/validate_memory_semantics.cpp b/source/val/validate_memory_semantics.cpp
index a0de18f..4c582f0 100644
--- a/source/val/validate_memory_semantics.cpp
+++ b/source/val/validate_memory_semantics.cpp
@@ -90,43 +90,17 @@
if (value & ~valid_bits) {
return _.diag(SPV_ERROR_INVALID_DATA, inst)
<< "For WebGPU only ImageMemory may be set for Memory "
- "Semantics "
- "of OpMemoryBarrier.";
+ "Semantics of OpMemoryBarrier.";
}
break;
default:
- // TODO(2723): Rewrite this to be in the style of above and simplify.
- valid_bits = SpvMemorySemanticsUniformMemoryMask |
- SpvMemorySemanticsWorkgroupMemoryMask |
- SpvMemorySemanticsImageMemoryMask |
- SpvMemorySemanticsOutputMemoryKHRMask |
- SpvMemorySemanticsMakeAvailableKHRMask |
- SpvMemorySemanticsMakeVisibleKHRMask;
- if (!spvOpcodeIsAtomicOp(inst->opcode())) {
- valid_bits |= SpvMemorySemanticsAcquireReleaseMask;
- }
-
- if (value & ~valid_bits) {
- if (spvOpcodeIsAtomicOp(inst->opcode())) {
+ if (spvOpcodeIsAtomicOp(inst->opcode())) {
+ if (value != 0) {
return _.diag(SPV_ERROR_INVALID_DATA, inst)
- << "WebGPU spec disallows, for OpAtomic*, any bit masks in "
- "Memory Semantics that are not UniformMemory, "
- "WorkgroupMemory, ImageMemory, or OutputMemoryKHR";
- } else {
- return _.diag(SPV_ERROR_INVALID_DATA, inst)
- << "WebGPU spec disallows any bit masks in Memory Semantics "
- "that are not AcquireRelease, UniformMemory, "
- "WorkgroupMemory, ImageMemory, OutputMemoryKHR, "
- "MakeAvailableKHR, or MakeVisibleKHR";
+ << "For WebGPU Memory no bits may be set for Memory "
+ "Semantics of OpAtomic* instructions.";
}
}
-
- if (!spvOpcodeIsAtomicOp(inst->opcode()) &&
- !(value & SpvMemorySemanticsAcquireReleaseMask)) {
- return _.diag(SPV_ERROR_INVALID_DATA, inst)
- << "WebGPU spec requires AcquireRelease to set in Memory "
- "Semantics.";
- }
break;
}
}
diff --git a/source/val/validate_scopes.cpp b/source/val/validate_scopes.cpp
index 47c4e45..70a65f7 100644
--- a/source/val/validate_scopes.cpp
+++ b/source/val/validate_scopes.cpp
@@ -246,6 +246,15 @@
}
break;
default:
+ if (spvOpcodeIsAtomicOp(inst->opcode())) {
+ if (value != SpvScopeQueueFamilyKHR) {
+ return _.diag(SPV_ERROR_INVALID_DATA, inst)
+ << spvOpcodeString(opcode)
+ << ": in WebGPU environment Memory Scope is limited to "
+ << "QueueFamilyKHR for OpAtomic* operations";
+ }
+ }
+
if (value != SpvScopeWorkgroup && value != SpvScopeInvocation &&
value != SpvScopeQueueFamilyKHR) {
return _.diag(SPV_ERROR_INVALID_DATA, inst)
diff --git a/test/val/val_atomics_test.cpp b/test/val/val_atomics_test.cpp
index 15887eb..57a1187 100644
--- a/test/val/val_atomics_test.cpp
+++ b/test/val/val_atomics_test.cpp
@@ -377,34 +377,34 @@
TEST_F(ValidateAtomics, AtomicLoadWebGPUSuccess) {
const std::string body = R"(
%val1 = OpAtomicLoad %u32 %u32_var %queuefamily %relaxed
-%val2 = OpAtomicLoad %u32 %u32_var %workgroup %relaxed
)";
CompileSuccessfully(GenerateWebGPUShaderCode(body), SPV_ENV_WEBGPU_0);
ASSERT_EQ(SPV_SUCCESS, ValidateInstructions(SPV_ENV_WEBGPU_0));
}
+TEST_F(ValidateAtomics, AtomicLoadWebGPUNonQueueFamilyFailure) {
+ const std::string body = R"(
+%val3 = OpAtomicLoad %u32 %u32_var %invocation %relaxed
+)";
+
+ CompileSuccessfully(GenerateWebGPUShaderCode(body), SPV_ENV_WEBGPU_0);
+ ASSERT_EQ(SPV_ERROR_INVALID_DATA, ValidateInstructions(SPV_ENV_WEBGPU_0));
+ EXPECT_THAT(getDiagnosticString(),
+ HasSubstr("Memory Scope is limited to QueueFamilyKHR for "
+ "OpAtomic* operations"));
+}
+
TEST_F(ValidateAtomics, AtomicLoadWebGPUNonRelaxedFailure) {
const std::string body = R"(
%val1 = OpAtomicLoad %u32 %u32_var %queuefamily %acquire
-%val2 = OpAtomicLoad %u32 %u32_var %workgroup %release
)";
CompileSuccessfully(GenerateWebGPUShaderCode(body), SPV_ENV_WEBGPU_0);
ASSERT_EQ(SPV_ERROR_INVALID_DATA, ValidateInstructions(SPV_ENV_WEBGPU_0));
EXPECT_THAT(getDiagnosticString(),
- HasSubstr("WebGPU spec disallows, for OpAtomic*, any bit masks"));
-}
-
-TEST_F(ValidateAtomics, AtomicLoadWebGPUSequentiallyConsistentFailure) {
- const std::string body = R"(
-%val3 = OpAtomicLoad %u32 %u32_var %invocation %sequentially_consistent
-)";
-
- CompileSuccessfully(GenerateWebGPUShaderCode(body), SPV_ENV_WEBGPU_0);
- ASSERT_EQ(SPV_ERROR_INVALID_DATA, ValidateInstructions(SPV_ENV_WEBGPU_0));
- EXPECT_THAT(getDiagnosticString(),
- HasSubstr("WebGPU spec disallows, for OpAtomic*, any bit masks"));
+ HasSubstr("no bits may be set for Memory Semantics of OpAtomic* "
+ "instructions"));
}
TEST_F(ValidateAtomics, VK_KHR_shader_atomic_int64Success) {
@@ -592,6 +592,17 @@
CompileSuccessfully(GenerateWebGPUShaderCode(body), SPV_ENV_WEBGPU_0);
ASSERT_EQ(SPV_SUCCESS, ValidateInstructions(SPV_ENV_WEBGPU_0));
}
+TEST_F(ValidateAtomics, AtomicStoreWebGPUNonQueueFamilyFailure) {
+ const std::string body = R"(
+OpAtomicStore %u32_var %workgroup %relaxed %u32_1
+)";
+
+ CompileSuccessfully(GenerateWebGPUShaderCode(body), SPV_ENV_WEBGPU_0);
+ ASSERT_EQ(SPV_ERROR_INVALID_DATA, ValidateInstructions(SPV_ENV_WEBGPU_0));
+ EXPECT_THAT(getDiagnosticString(),
+ HasSubstr("Memory Scope is limited to QueueFamilyKHR for "
+ "OpAtomic* operations"));
+}
TEST_F(ValidateAtomics, AtomicStoreWebGPUNonRelaxedFailure) {
const std::string body = R"(
@@ -601,18 +612,8 @@
CompileSuccessfully(GenerateWebGPUShaderCode(body), SPV_ENV_WEBGPU_0);
ASSERT_EQ(SPV_ERROR_INVALID_DATA, ValidateInstructions(SPV_ENV_WEBGPU_0));
EXPECT_THAT(getDiagnosticString(),
- HasSubstr("WebGPU spec disallows, for OpAtomic*, any bit masks"));
-}
-
-TEST_F(ValidateAtomics, AtomicStoreWebGPUSequentiallyConsistent) {
- const std::string body = R"(
-OpAtomicStore %u32_var %queuefamily %sequentially_consistent %u32_1
-)";
-
- CompileSuccessfully(GenerateWebGPUShaderCode(body), SPV_ENV_WEBGPU_0);
- ASSERT_EQ(SPV_ERROR_INVALID_DATA, ValidateInstructions(SPV_ENV_WEBGPU_0));
- EXPECT_THAT(getDiagnosticString(),
- HasSubstr("WebGPU spec disallows, for OpAtomic*, any bit masks"));
+ HasSubstr("no bits may be set for Memory Semantics of OpAtomic* "
+ "instructions"));
}
TEST_F(ValidateAtomics, AtomicStoreWrongPointerType) {
@@ -1919,11 +1920,9 @@
CompileSuccessfully(GenerateWebGPUShaderCode(body), SPV_ENV_WEBGPU_0);
EXPECT_EQ(SPV_ERROR_INVALID_DATA, ValidateInstructions(SPV_ENV_WEBGPU_0));
- EXPECT_THAT(
- getDiagnosticString(),
- HasSubstr("AtomicLoad: in WebGPU environment Memory Scope is limited to "
- "Workgroup, Invocation, and QueueFamilyKHR\n"
- " %34 = OpAtomicLoad %uint %29 %uint_0_0 %uint_0_1\n"));
+ EXPECT_THAT(getDiagnosticString(),
+ HasSubstr("in WebGPU environment Memory Scope is limited to "
+ "QueueFamilyKHR for OpAtomic* operations"));
}
TEST_F(ValidateAtomics, WebGPUDeviceMemoryScopeBad) {
@@ -1933,20 +1932,21 @@
CompileSuccessfully(GenerateWebGPUShaderCode(body), SPV_ENV_WEBGPU_0);
EXPECT_EQ(SPV_ERROR_INVALID_DATA, ValidateInstructions(SPV_ENV_WEBGPU_0));
- EXPECT_THAT(
- getDiagnosticString(),
- HasSubstr("AtomicLoad: in WebGPU environment Memory Scope is limited to "
- "Workgroup, Invocation, and QueueFamilyKHR\n"
- " %34 = OpAtomicLoad %uint %29 %uint_1_0 %uint_0_1\n"));
+ EXPECT_THAT(getDiagnosticString(),
+ HasSubstr("in WebGPU environment Memory Scope is limited to "
+ "QueueFamilyKHR for OpAtomic* operations"));
}
-TEST_F(ValidateAtomics, WebGPUWorkgroupMemoryScopeGood) {
+TEST_F(ValidateAtomics, WebGPUWorkgroupMemoryScopeBad) {
const std::string body = R"(
%val1 = OpAtomicLoad %u32 %u32_var %workgroup %relaxed
)";
CompileSuccessfully(GenerateWebGPUShaderCode(body), SPV_ENV_WEBGPU_0);
- EXPECT_EQ(SPV_SUCCESS, ValidateInstructions(SPV_ENV_WEBGPU_0));
+ EXPECT_EQ(SPV_ERROR_INVALID_DATA, ValidateInstructions(SPV_ENV_WEBGPU_0));
+ EXPECT_THAT(getDiagnosticString(),
+ HasSubstr("in WebGPU environment Memory Scope is limited to "
+ "QueueFamilyKHR for OpAtomic* operations"));
}
TEST_F(ValidateAtomics, WebGPUSubgroupMemoryScopeBad) {
@@ -1956,20 +1956,21 @@
CompileSuccessfully(GenerateWebGPUShaderCode(body), SPV_ENV_WEBGPU_0);
EXPECT_EQ(SPV_ERROR_INVALID_DATA, ValidateInstructions(SPV_ENV_WEBGPU_0));
- EXPECT_THAT(
- getDiagnosticString(),
- HasSubstr("AtomicLoad: in WebGPU environment Memory Scope is limited to "
- "Workgroup, Invocation, and QueueFamilyKHR\n"
- " %34 = OpAtomicLoad %uint %29 %uint_3 %uint_0_1\n"));
+ EXPECT_THAT(getDiagnosticString(),
+ HasSubstr("in WebGPU environment Memory Scope is limited to "
+ "QueueFamilyKHR for OpAtomic* operations"));
}
-TEST_F(ValidateAtomics, WebGPUInvocationMemoryScopeGood) {
+TEST_F(ValidateAtomics, WebGPUInvocationMemoryScopeBad) {
const std::string body = R"(
%val1 = OpAtomicLoad %u32 %u32_var %invocation %relaxed
)";
CompileSuccessfully(GenerateWebGPUShaderCode(body), SPV_ENV_WEBGPU_0);
- EXPECT_EQ(SPV_SUCCESS, ValidateInstructions(SPV_ENV_WEBGPU_0));
+ EXPECT_EQ(SPV_ERROR_INVALID_DATA, ValidateInstructions(SPV_ENV_WEBGPU_0));
+ EXPECT_THAT(getDiagnosticString(),
+ HasSubstr("in WebGPU environment Memory Scope is limited to "
+ "QueueFamilyKHR for OpAtomic* operations"));
}
TEST_F(ValidateAtomics, WebGPUQueueFamilyMemoryScopeGood) {
