Remove check for SpvCapabilityAtomicStorage (#2243)
Per conversation on
https://github.com/KhronosGroup/glslang/issues/1618 and other places.
diff --git a/source/val/validate_memory_semantics.cpp b/source/val/validate_memory_semantics.cpp
index ec6df38..f90b5e4 100644
--- a/source/val/validate_memory_semantics.cpp
+++ b/source/val/validate_memory_semantics.cpp
@@ -119,15 +119,8 @@
<< ": Memory Semantics UniformMemory requires capability Shader";
}
- // Disabling this check until
- // https://github.com/KhronosGroup/glslang/issues/1618 is resolved.
- // if (value & SpvMemorySemanticsAtomicCounterMemoryMask &&
- // !_.HasCapability(SpvCapabilityAtomicStorage)) {
- // return _.diag(SPV_ERROR_INVALID_DATA, inst)
- // << spvOpcodeString(opcode)
- // << ": Memory Semantics AtomicCounterMemory requires capability "
- // "AtomicStorage";
- // }
+ // Checking for SpvCapabilityAtomicStorage is intentionally not done here. See
+ // https://github.com/KhronosGroup/glslang/issues/1618 for the reasoning why.
if (value & (SpvMemorySemanticsMakeAvailableKHRMask |
SpvMemorySemanticsMakeVisibleKHRMask)) {
diff --git a/test/val/val_atomics_test.cpp b/test/val/val_atomics_test.cpp
index 98a2149..03bbe82 100644
--- a/test/val/val_atomics_test.cpp
+++ b/test/val/val_atomics_test.cpp
@@ -1158,24 +1158,18 @@
"requires capability Shader"));
}
-// Disabling this test until
-// https://github.com/KhronosGroup/glslang/issues/1618 is resolved.
-// TEST_F(ValidateAtomics, AtomicCounterMemorySemanticsNoCapability) {
-// const std::string body = R"(
-// OpAtomicStore %u32_var %device %relaxed %u32_1
-//%val1 = OpAtomicIIncrement %u32 %u32_var %device
-//%acquire_release_atomic_counter_workgroup
-//)";
-//
-// CompileSuccessfully(GenerateKernelCode(body));
-// ASSERT_EQ(SPV_ERROR_INVALID_DATA, ValidateInstructions());
-// EXPECT_THAT(
-// getDiagnosticString(),
-// HasSubstr("AtomicIIncrement: Memory Semantics AtomicCounterMemory "
-// "requires capability AtomicStorage\n %40 = OpAtomicIIncrement
-// "
-// "%uint %30 %uint_1_0 %uint_1288\n"));
-//}
+// Lack of the AtomicStorage capability is intentionally ignored, see
+// https://github.com/KhronosGroup/glslang/issues/1618 for the reasoning why.
+TEST_F(ValidateAtomics, AtomicCounterMemorySemanticsNoCapability) {
+ const std::string body = R"(
+ OpAtomicStore %u32_var %device %relaxed %u32_1
+%val1 = OpAtomicIIncrement %u32 %u32_var %device
+%acquire_release_atomic_counter_workgroup
+)";
+
+ CompileSuccessfully(GenerateKernelCode(body));
+ ASSERT_EQ(SPV_SUCCESS, ValidateInstructions());
+}
TEST_F(ValidateAtomics, AtomicCounterMemorySemanticsWithCapability) {
const std::string body = R"(