Merging two ValidateMemoryScope implementations (#2132)

Fixes #2125
This commit is contained in:
Ryan Harrison 2018-11-29 14:51:17 -05:00 committed by GitHub
parent 2d2a512691
commit 2cd040b0d3
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
5 changed files with 78 additions and 86 deletions

View File

@ -21,46 +21,12 @@
#include "source/spirv_target_env.h"
#include "source/util/bitutils.h"
#include "source/val/instruction.h"
#include "source/val/validate_scopes.h"
#include "source/val/validation_state.h"
namespace spvtools {
namespace val {
// Validates Memory Scope operand.
spv_result_t ValidateMemoryScope(ValidationState_t& _, const Instruction* inst,
uint32_t id) {
const SpvOp opcode = inst->opcode();
bool is_int32 = false, is_const_int32 = false;
uint32_t value = 0;
std::tie(is_int32, is_const_int32, value) = _.EvalInt32IfConst(id);
if (!is_int32) {
return _.diag(SPV_ERROR_INVALID_DATA, inst)
<< spvOpcodeString(opcode) << ": expected Scope to be 32-bit int";
}
if (!is_const_int32) {
return SPV_SUCCESS;
}
#if 0
// TODO(atgoo@github.com): this check fails Vulkan CTS, reenable once fixed.
if (spvIsVulkanEnv(_.context()->target_env)) {
if (value != SpvScopeDevice && value != SpvScopeWorkgroup &&
value != SpvScopeInvocation) {
return _.diag(SPV_ERROR_INVALID_DATA, inst)
<< spvOpcodeString(opcode)
<< ": in Vulkan environment memory scope is limited to Device, "
"Workgroup and Invocation";
}
}
#endif
// TODO(atgoo@github.com) Add checks for OpenCL and OpenGL environments.
return SPV_SUCCESS;
}
// Validates a Memory Semantics operand.
spv_result_t ValidateMemorySemantics(ValidationState_t& _,
const Instruction* inst,

View File

@ -31,46 +31,6 @@ namespace spvtools {
namespace val {
namespace {
// Validates Memory Scope operand.
spv_result_t ValidateMemoryScope(ValidationState_t& _, const Instruction* inst,
uint32_t id) {
const SpvOp opcode = inst->opcode();
bool is_int32 = false, is_const_int32 = false;
uint32_t value = 0;
std::tie(is_int32, is_const_int32, value) = _.EvalInt32IfConst(id);
if (!is_int32) {
return _.diag(SPV_ERROR_INVALID_DATA, inst)
<< spvOpcodeString(opcode)
<< ": expected Memory Scope to be a 32-bit int";
}
if (!is_const_int32) {
return SPV_SUCCESS;
}
if (spvIsVulkanEnv(_.context()->target_env)) {
if (value == SpvScopeCrossDevice) {
return _.diag(SPV_ERROR_INVALID_DATA, inst)
<< spvOpcodeString(opcode)
<< ": in Vulkan environment, Memory Scope cannot be CrossDevice";
}
if (_.context()->target_env == SPV_ENV_VULKAN_1_0 &&
value != SpvScopeDevice && value != SpvScopeWorkgroup &&
value != SpvScopeInvocation) {
return _.diag(SPV_ERROR_INVALID_DATA, inst)
<< spvOpcodeString(opcode)
<< ": in Vulkan 1.0 environment Memory Scope is limited to "
"Device, "
"Workgroup and Invocation";
}
}
// TODO(atgoo@github.com) Add checks for OpenCL and OpenGL environments.
return SPV_SUCCESS;
}
// Validates Memory Semantics operand.
spv_result_t ValidateMemorySemantics(ValidationState_t& _,
const Instruction* inst, uint32_t id) {

View File

@ -100,5 +100,56 @@ spv_result_t ValidateExecutionScope(ValidationState_t& _,
return SPV_SUCCESS;
}
spv_result_t ValidateMemoryScope(ValidationState_t& _, const Instruction* inst,
uint32_t scope) {
const SpvOp opcode = inst->opcode();
bool is_int32 = false, is_const_int32 = false;
uint32_t value = 0;
std::tie(is_int32, is_const_int32, value) = _.EvalInt32IfConst(scope);
if (!is_int32) {
return _.diag(SPV_ERROR_INVALID_DATA, inst)
<< spvOpcodeString(opcode)
<< ": expected Memory Scope to be a 32-bit int";
}
if (!is_const_int32) {
return SPV_SUCCESS;
}
// Vulkan Specific rules
if (spvIsVulkanEnv(_.context()->target_env)) {
if (value == SpvScopeCrossDevice) {
return _.diag(SPV_ERROR_INVALID_DATA, inst)
<< spvOpcodeString(opcode)
<< ": in Vulkan environment, Memory Scope cannot be CrossDevice";
}
// Vulkan 1.0 specifc rules
if (_.context()->target_env == SPV_ENV_VULKAN_1_0 &&
value != SpvScopeDevice && value != SpvScopeWorkgroup &&
value != SpvScopeInvocation) {
return _.diag(SPV_ERROR_INVALID_DATA, inst)
<< spvOpcodeString(opcode)
<< ": in Vulkan 1.0 environment Memory Scope is limited to "
"Device, "
"Workgroup and Invocation";
}
// Vulkan 1.1 specifc rules
if (_.context()->target_env == SPV_ENV_VULKAN_1_0 &&
value != SpvScopeDevice && value != SpvScopeWorkgroup &&
value != SpvScopeSubgroup && value != SpvScopeInvocation) {
return _.diag(SPV_ERROR_INVALID_DATA, inst)
<< spvOpcodeString(opcode)
<< ": in Vulkan 1.1 environment Memory Scope is limited to "
"Device, "
"Workgroup and Invocation";
}
}
// TODO(atgoo@github.com) Add checks for OpenCL and OpenGL environments.
return SPV_SUCCESS;
}
} // namespace val
} // namespace spvtools

View File

@ -23,5 +23,8 @@ namespace val {
spv_result_t ValidateExecutionScope(ValidationState_t& _,
const Instruction* inst, uint32_t scope);
spv_result_t ValidateMemoryScope(ValidationState_t& _, const Instruction* inst,
uint32_t scope);
} // namespace val
} // namespace spvtools

View File

@ -404,8 +404,10 @@ TEST_F(ValidateAtomics, AtomicLoadWrongScopeType) {
CompileSuccessfully(GenerateKernelCode(body));
ASSERT_EQ(SPV_ERROR_INVALID_DATA, ValidateInstructions());
EXPECT_THAT(getDiagnosticString(),
HasSubstr("AtomicLoad: expected Scope to be 32-bit int"));
EXPECT_THAT(
getDiagnosticString(),
HasSubstr("AtomicLoad: expected Memory Scope to be a 32-bit int\n %40 = "
"OpAtomicLoad %float %28 %float_1 %uint_0_1\n"));
}
TEST_F(ValidateAtomics, AtomicLoadWrongMemorySemanticsType) {
@ -535,8 +537,10 @@ OpAtomicStore %f32_var %f32_1 %relaxed %f32_1
CompileSuccessfully(GenerateKernelCode(body));
ASSERT_EQ(SPV_ERROR_INVALID_DATA, ValidateInstructions());
EXPECT_THAT(getDiagnosticString(),
HasSubstr("AtomicStore: expected Scope to be 32-bit int"));
EXPECT_THAT(
getDiagnosticString(),
HasSubstr("AtomicStore: expected Memory Scope to be a 32-bit int\n "
"OpAtomicStore %28 %float_1 %uint_0_1 %float_1\n"));
}
TEST_F(ValidateAtomics, AtomicStoreWrongMemorySemanticsType) {
@ -648,8 +652,11 @@ OpAtomicStore %f32_var %device %relaxed %f32_1
CompileSuccessfully(GenerateKernelCode(body));
ASSERT_EQ(SPV_ERROR_INVALID_DATA, ValidateInstructions());
EXPECT_THAT(getDiagnosticString(),
HasSubstr("AtomicExchange: expected Scope to be 32-bit int"));
EXPECT_THAT(
getDiagnosticString(),
HasSubstr(
"AtomicExchange: expected Memory Scope to be a 32-bit int\n %40 = "
"OpAtomicExchange %float %28 %float_1 %uint_0_1 %float_0\n"));
}
TEST_F(ValidateAtomics, AtomicExchangeWrongMemorySemanticsType) {
@ -762,7 +769,9 @@ OpAtomicStore %f32_var %device %relaxed %f32_1
ASSERT_EQ(SPV_ERROR_INVALID_DATA, ValidateInstructions());
EXPECT_THAT(
getDiagnosticString(),
HasSubstr("AtomicCompareExchange: expected Scope to be 32-bit int"));
HasSubstr("AtomicCompareExchange: expected Memory Scope to be a 32-bit "
"int\n %40 = OpAtomicCompareExchange %float %28 %float_1 "
"%uint_0_1 %uint_0_1 %float_0 %float_0\n"));
}
TEST_F(ValidateAtomics, AtomicCompareExchangeWrongMemorySemanticsType1) {
@ -942,9 +951,11 @@ TEST_F(ValidateAtomics, AtomicFlagTestAndSetWrongScopeType) {
CompileSuccessfully(GenerateKernelCode(body));
ASSERT_EQ(SPV_ERROR_INVALID_DATA, ValidateInstructions());
EXPECT_THAT(getDiagnosticString(),
HasSubstr("AtomicFlagTestAndSet: "
"expected Scope to be 32-bit int"));
EXPECT_THAT(
getDiagnosticString(),
HasSubstr(
"AtomicFlagTestAndSet: expected Memory Scope to be a 32-bit int\n "
"%40 = OpAtomicFlagTestAndSet %bool %30 %ulong_1 %uint_0_1\n"));
}
TEST_F(ValidateAtomics, AtomicFlagTestAndSetWrongMemorySemanticsType) {
@ -1017,7 +1028,8 @@ OpAtomicFlagClear %u32_var %u64_1 %relaxed
CompileSuccessfully(GenerateKernelCode(body));
ASSERT_EQ(SPV_ERROR_INVALID_DATA, ValidateInstructions());
EXPECT_THAT(getDiagnosticString(),
HasSubstr("AtomicFlagClear: expected Scope to be 32-bit int"));
HasSubstr("AtomicFlagClear: expected Memory Scope to be a 32-bit "
"int\n OpAtomicFlagClear %30 %ulong_1 %uint_0_1\n"));
}
TEST_F(ValidateAtomics, AtomicFlagClearWrongMemorySemanticsType) {