Validate Volatile memory semantics bit (#2672)
* Can only be used with Vulkan memory model
* Can only be used with atomics
* Bit setting must match for compare exchange opcodes
* Updated memory semantics checks to allow constant instructions
generally with CooperativeMatrixNV
diff --git a/source/val/validate_atomics.cpp b/source/val/validate_atomics.cpp
index 38c7053..e9e4960 100644
--- a/source/val/validate_atomics.cpp
+++ b/source/val/validate_atomics.cpp
@@ -175,13 +175,37 @@
return error;
}
- if (auto error = ValidateMemorySemantics(_, inst, operand_index++))
+ const auto equal_semantics_index = operand_index++;
+ if (auto error = ValidateMemorySemantics(_, inst, equal_semantics_index))
return error;
if (opcode == SpvOpAtomicCompareExchange ||
opcode == SpvOpAtomicCompareExchangeWeak) {
- if (auto error = ValidateMemorySemantics(_, inst, operand_index++))
+ const auto unequal_semantics_index = operand_index++;
+ if (auto error =
+ ValidateMemorySemantics(_, inst, unequal_semantics_index))
return error;
+
+ // Volatile bits must match for equal and unequal semantics. Previous
+ // checks guarantee they are 32-bit constants, but we need to recheck
+ // whether they are evaluatable constants.
+ bool is_int32 = false;
+ bool is_equal_const = false;
+ bool is_unequal_const = false;
+ uint32_t equal_value = 0;
+ uint32_t unequal_value = 0;
+ std::tie(is_int32, is_equal_const, equal_value) = _.EvalInt32IfConst(
+ inst->GetOperandAs<uint32_t>(equal_semantics_index));
+ std::tie(is_int32, is_unequal_const, unequal_value) =
+ _.EvalInt32IfConst(
+ inst->GetOperandAs<uint32_t>(unequal_semantics_index));
+ if (is_equal_const && is_unequal_const &&
+ ((equal_value & SpvMemorySemanticsVolatileMask) ^
+ (unequal_value & SpvMemorySemanticsVolatileMask))) {
+ return _.diag(SPV_ERROR_INVALID_ID, inst)
+ << "Volatile mask setting must match for Equal and Unequal "
+ "memory semantics";
+ }
}
if (opcode == SpvOpAtomicStore) {
diff --git a/source/val/validate_memory_semantics.cpp b/source/val/validate_memory_semantics.cpp
index 31154f5..0088cdd 100644
--- a/source/val/validate_memory_semantics.cpp
+++ b/source/val/validate_memory_semantics.cpp
@@ -39,11 +39,20 @@
}
if (!is_const_int32) {
- if (_.HasCapability(SpvCapabilityShader)) {
+ if (_.HasCapability(SpvCapabilityShader) &&
+ !_.HasCapability(SpvCapabilityCooperativeMatrixNV)) {
return _.diag(SPV_ERROR_INVALID_DATA, inst)
<< "Memory Semantics ids must be OpConstant when Shader "
"capability is present";
}
+
+ if (_.HasCapability(SpvCapabilityShader) &&
+ _.HasCapability(SpvCapabilityCooperativeMatrixNV) &&
+ !spvOpcodeIsConstant(_.GetIdOpcode(id))) {
+ return _.diag(SPV_ERROR_INVALID_DATA, inst)
+ << "Memory Semantics must be a constant instruction when "
+ "CooperativeMatrixNV capability is present";
+ }
return SPV_SUCCESS;
}
@@ -127,6 +136,21 @@
<< "VulkanMemoryModelKHR";
}
+ if (value & SpvMemorySemanticsVolatileMask) {
+ if (!_.HasCapability(SpvCapabilityVulkanMemoryModelKHR)) {
+ return _.diag(SPV_ERROR_INVALID_DATA, inst)
+ << spvOpcodeString(opcode)
+ << ": Memory Semantics Volatile requires capability "
+ "VulkanMemoryModelKHR";
+ }
+
+ if (!spvOpcodeIsAtomicOp(inst->opcode())) {
+ return _.diag(SPV_ERROR_INVALID_DATA, inst)
+ << "Memory Semantics Volatile can only be used with atomic "
+ "instructions";
+ }
+ }
+
if (value & SpvMemorySemanticsUniformMemoryMask &&
!_.HasCapability(SpvCapabilityShader)) {
return _.diag(SPV_ERROR_INVALID_DATA, inst)
diff --git a/test/val/val_atomics_test.cpp b/test/val/val_atomics_test.cpp
index 58b2846..f130e02 100644
--- a/test/val/val_atomics_test.cpp
+++ b/test/val/val_atomics_test.cpp
@@ -1998,6 +1998,153 @@
"AtomicCompareExchangeWeak requires SPIR-V version 1.3 or earlier"));
}
+TEST_F(ValidateAtomics, CompareExchangeVolatileMatch) {
+ const std::string spirv = R"(
+OpCapability Shader
+OpCapability VulkanMemoryModelKHR
+OpCapability Linkage
+OpExtension "SPV_KHR_vulkan_memory_model"
+OpMemoryModel Logical VulkanKHR
+%void = OpTypeVoid
+%int = OpTypeInt 32 0
+%int_0 = OpConstant %int 0
+%int_1 = OpConstant %int 1
+%workgroup = OpConstant %int 2
+%volatile = OpConstant %int 32768
+%ptr_wg_int = OpTypePointer Workgroup %int
+%wg_var = OpVariable %ptr_wg_int Workgroup
+%void_fn = OpTypeFunction %void
+%func = OpFunction %void None %void_fn
+%entry = OpLabel
+%cmp_ex = OpAtomicCompareExchange %int %wg_var %workgroup %volatile %volatile %int_0 %int_1
+OpReturn
+OpFunctionEnd
+)";
+
+ CompileSuccessfully(spirv);
+ EXPECT_EQ(SPV_SUCCESS, ValidateInstructions());
+}
+
+TEST_F(ValidateAtomics, CompareExchangeVolatileMismatch) {
+ const std::string spirv = R"(
+OpCapability Shader
+OpCapability VulkanMemoryModelKHR
+OpCapability Linkage
+OpExtension "SPV_KHR_vulkan_memory_model"
+OpMemoryModel Logical VulkanKHR
+%void = OpTypeVoid
+%int = OpTypeInt 32 0
+%int_0 = OpConstant %int 0
+%int_1 = OpConstant %int 1
+%workgroup = OpConstant %int 2
+%volatile = OpConstant %int 32768
+%non_volatile = OpConstant %int 0
+%ptr_wg_int = OpTypePointer Workgroup %int
+%wg_var = OpVariable %ptr_wg_int Workgroup
+%void_fn = OpTypeFunction %void
+%func = OpFunction %void None %void_fn
+%entry = OpLabel
+%cmp_ex = OpAtomicCompareExchange %int %wg_var %workgroup %non_volatile %volatile %int_0 %int_1
+OpReturn
+OpFunctionEnd
+)";
+
+ CompileSuccessfully(spirv);
+ EXPECT_EQ(SPV_ERROR_INVALID_ID, ValidateInstructions());
+ EXPECT_THAT(getDiagnosticString(),
+ HasSubstr("Volatile mask setting must match for Equal and "
+ "Unequal memory semantics"));
+}
+
+TEST_F(ValidateAtomics, CompareExchangeVolatileMismatchCooperativeMatrix) {
+ const std::string spirv = R"(
+OpCapability Shader
+OpCapability VulkanMemoryModelKHR
+OpCapability Linkage
+OpCapability CooperativeMatrixNV
+OpExtension "SPV_KHR_vulkan_memory_model"
+OpExtension "SPV_NV_cooperative_matrix"
+OpMemoryModel Logical VulkanKHR
+%void = OpTypeVoid
+%int = OpTypeInt 32 0
+%int_0 = OpConstant %int 0
+%int_1 = OpConstant %int 1
+%workgroup = OpConstant %int 2
+%volatile = OpSpecConstant %int 32768
+%non_volatile = OpSpecConstant %int 32768
+%ptr_wg_int = OpTypePointer Workgroup %int
+%wg_var = OpVariable %ptr_wg_int Workgroup
+%void_fn = OpTypeFunction %void
+%func = OpFunction %void None %void_fn
+%entry = OpLabel
+%cmp_ex = OpAtomicCompareExchange %int %wg_var %workgroup %volatile %non_volatile %int_0 %int_1
+OpReturn
+OpFunctionEnd
+)";
+
+ // This is ok because we cannot evaluate the spec constant defaults.
+ CompileSuccessfully(spirv);
+ EXPECT_EQ(SPV_SUCCESS, ValidateInstructions());
+}
+
+TEST_F(ValidateAtomics, VolatileRequiresVulkanMemoryModel) {
+ const std::string spirv = R"(
+OpCapability Shader
+OpCapability Linkage
+OpMemoryModel Logical GLSL450
+%void = OpTypeVoid
+%int = OpTypeInt 32 0
+%int_0 = OpConstant %int 0
+%int_1 = OpConstant %int 1
+%workgroup = OpConstant %int 2
+%volatile = OpConstant %int 32768
+%ptr_wg_int = OpTypePointer Workgroup %int
+%wg_var = OpVariable %ptr_wg_int Workgroup
+%void_fn = OpTypeFunction %void
+%func = OpFunction %void None %void_fn
+%entry = OpLabel
+%ld = OpAtomicLoad %int %wg_var %workgroup %volatile
+OpReturn
+OpFunctionEnd
+)";
+
+ CompileSuccessfully(spirv);
+ EXPECT_EQ(SPV_ERROR_INVALID_DATA, ValidateInstructions());
+ EXPECT_THAT(getDiagnosticString(),
+ HasSubstr("Memory Semantics Volatile requires capability "
+ "VulkanMemoryModelKHR"));
+}
+
+TEST_F(ValidateAtomics, CooperativeMatrixSemanticsMustBeConstant) {
+ const std::string spirv = R"(
+OpCapability Shader
+OpCapability Linkage
+OpCapability CooperativeMatrixNV
+OpExtension "SPV_NV_cooperative_matrix"
+OpMemoryModel Logical GLSL450
+%void = OpTypeVoid
+%int = OpTypeInt 32 0
+%int_0 = OpConstant %int 0
+%int_1 = OpConstant %int 1
+%workgroup = OpConstant %int 2
+%undef = OpUndef %int
+%ptr_wg_int = OpTypePointer Workgroup %int
+%wg_var = OpVariable %ptr_wg_int Workgroup
+%void_fn = OpTypeFunction %void
+%func = OpFunction %void None %void_fn
+%entry = OpLabel
+%ld = OpAtomicLoad %int %wg_var %workgroup %undef
+OpReturn
+OpFunctionEnd
+)";
+
+ CompileSuccessfully(spirv);
+ EXPECT_EQ(SPV_ERROR_INVALID_DATA, ValidateInstructions());
+ EXPECT_THAT(getDiagnosticString(),
+ HasSubstr("Memory Semantics must be a constant instruction when "
+ "CooperativeMatrixNV capability is present"));
+}
+
} // namespace
} // namespace val
} // namespace spvtools
diff --git a/test/val/val_barriers_test.cpp b/test/val/val_barriers_test.cpp
index cf81e42..b28e51a 100644
--- a/test/val/val_barriers_test.cpp
+++ b/test/val/val_barriers_test.cpp
@@ -1361,6 +1361,115 @@
EXPECT_EQ(SPV_SUCCESS, ValidateInstructions(SPV_ENV_UNIVERSAL_1_3));
}
+TEST_F(ValidateBarriers, VolatileMemoryBarrier) {
+ const std::string text = R"(
+OpCapability Shader
+OpCapability VulkanMemoryModelKHR
+OpCapability VulkanMemoryModelDeviceScopeKHR
+OpCapability Linkage
+OpExtension "SPV_KHR_vulkan_memory_model"
+OpMemoryModel Logical VulkanKHR
+%void = OpTypeVoid
+%int = OpTypeInt 32 0
+%device = OpConstant %int 1
+%semantics = OpConstant %int 32768
+%functy = OpTypeFunction %void
+%func = OpFunction %void None %functy
+%1 = OpLabel
+OpMemoryBarrier %device %semantics
+OpReturn
+OpFunctionEnd
+)";
+
+ CompileSuccessfully(text);
+ EXPECT_EQ(SPV_ERROR_INVALID_DATA, ValidateInstructions());
+ EXPECT_THAT(getDiagnosticString(),
+ HasSubstr("Memory Semantics Volatile can only be used with "
+ "atomic instructions"));
+}
+
+TEST_F(ValidateBarriers, VolatileControlBarrier) {
+ const std::string text = R"(
+OpCapability Shader
+OpCapability VulkanMemoryModelKHR
+OpCapability VulkanMemoryModelDeviceScopeKHR
+OpCapability Linkage
+OpExtension "SPV_KHR_vulkan_memory_model"
+OpMemoryModel Logical VulkanKHR
+%void = OpTypeVoid
+%int = OpTypeInt 32 0
+%device = OpConstant %int 1
+%semantics = OpConstant %int 32768
+%functy = OpTypeFunction %void
+%func = OpFunction %void None %functy
+%1 = OpLabel
+OpControlBarrier %device %device %semantics
+OpReturn
+OpFunctionEnd
+)";
+
+ CompileSuccessfully(text);
+ EXPECT_EQ(SPV_ERROR_INVALID_DATA, ValidateInstructions());
+ EXPECT_THAT(getDiagnosticString(),
+ HasSubstr("Memory Semantics Volatile can only be used with "
+ "atomic instructions"));
+}
+
+TEST_F(ValidateBarriers, CooperativeMatrixSpecConstantVolatile) {
+ const std::string text = R"(
+OpCapability Shader
+OpCapability VulkanMemoryModelKHR
+OpCapability VulkanMemoryModelDeviceScopeKHR
+OpCapability CooperativeMatrixNV
+OpCapability Linkage
+OpExtension "SPV_KHR_vulkan_memory_model"
+OpExtension "SPV_NV_cooperative_matrix"
+OpMemoryModel Logical VulkanKHR
+%void = OpTypeVoid
+%int = OpTypeInt 32 0
+%device = OpConstant %int 1
+%semantics = OpSpecConstant %int 32768
+%functy = OpTypeFunction %void
+%func = OpFunction %void None %functy
+%1 = OpLabel
+OpControlBarrier %device %device %semantics
+OpReturn
+OpFunctionEnd
+)";
+
+ CompileSuccessfully(text);
+ EXPECT_EQ(SPV_SUCCESS, ValidateInstructions());
+}
+
+TEST_F(ValidateBarriers, CooperativeMatrixNonConstantSemantics) {
+ const std::string text = R"(
+OpCapability Shader
+OpCapability VulkanMemoryModelKHR
+OpCapability VulkanMemoryModelDeviceScopeKHR
+OpCapability CooperativeMatrixNV
+OpCapability Linkage
+OpExtension "SPV_KHR_vulkan_memory_model"
+OpExtension "SPV_NV_cooperative_matrix"
+OpMemoryModel Logical VulkanKHR
+%void = OpTypeVoid
+%int = OpTypeInt 32 0
+%device = OpConstant %int 1
+%semantics = OpUndef %int
+%functy = OpTypeFunction %void
+%func = OpFunction %void None %functy
+%1 = OpLabel
+OpControlBarrier %device %device %semantics
+OpReturn
+OpFunctionEnd
+)";
+
+ CompileSuccessfully(text);
+ EXPECT_EQ(SPV_ERROR_INVALID_DATA, ValidateInstructions());
+ EXPECT_THAT(getDiagnosticString(),
+ HasSubstr("Memory Semantics must be a constant instruction when "
+ "CooperativeMatrixNV capability is present"));
+}
+
} // namespace
} // namespace val
} // namespace spvtools
