AuroraMiddleware/SPIRV-Tools
1
0
Fork 0
mirror of https://github.com/KhronosGroup/SPIRV-Tools synced 2024-11-26 21:30:07 +00:00
Code Issues Packages Projects Releases Wiki Activity

spirv-val: Refactor of atomic pass (#4200)

Browse Source
This commit is contained in:
sfricke-samsung 2021-03-19 06:23:02 -07:00 committed by GitHub
parent 8f421ced3e
commit f2a19b0150
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
2 changed files with 149 additions and 117 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

198
source/val/validate_atomics.cpp
View File

@ -47,6 +47,70 @@ bool IsStorageClassAllowedByUniversalRules(uint32_t storage_class) {
}
}
bool HasReturnType(uint32_t opcode) {
switch (opcode) {
case SpvOpAtomicStore:
case SpvOpAtomicFlagClear:
return false;
break;
default:
return true;
}
}
bool HasOnlyFloatReturnType(uint32_t opcode) {
switch (opcode) {
case SpvOpAtomicFAddEXT:
return true;
break;
default:
return false;
}
}
bool HasOnlyIntReturnType(uint32_t opcode) {
switch (opcode) {
case SpvOpAtomicCompareExchange:
case SpvOpAtomicCompareExchangeWeak:
case SpvOpAtomicIIncrement:
case SpvOpAtomicIDecrement:
case SpvOpAtomicIAdd:
case SpvOpAtomicISub:
case SpvOpAtomicSMin:
case SpvOpAtomicUMin:
case SpvOpAtomicSMax:
case SpvOpAtomicUMax:
case SpvOpAtomicAnd:
case SpvOpAtomicOr:
case SpvOpAtomicXor:
return true;
break;
default:
return false;
}
}
bool HasIntOrFloatReturnType(uint32_t opcode) {
switch (opcode) {
case SpvOpAtomicLoad:
case SpvOpAtomicExchange:
return true;
break;
default:
return false;
}
}
bool HasOnlyBoolReturnType(uint32_t opcode) {
switch (opcode) {
case SpvOpAtomicFlagTestAndSet:
return true;
break;
default:
return false;
}
}
} // namespace
namespace spvtools {
@ -55,12 +119,6 @@ namespace val {
// Validates correctness of atomic instructions.
spv_result_t AtomicsPass(ValidationState_t& _, const Instruction* inst) {
const SpvOp opcode = inst->opcode();
const uint32_t result_type = inst->type_id();
bool is_atomic_float_opcode = false;
if (opcode == SpvOpAtomicLoad || opcode == SpvOpAtomicStore ||
opcode == SpvOpAtomicFAddEXT || opcode == SpvOpAtomicExchange) {
is_atomic_float_opcode = true;
}
switch (opcode) {
case SpvOpAtomicLoad:
case SpvOpAtomicStore:
@ -81,82 +139,38 @@ spv_result_t AtomicsPass(ValidationState_t& _, const Instruction* inst) {
case SpvOpAtomicXor:
case SpvOpAtomicFlagTestAndSet:
case SpvOpAtomicFlagClear: {
if (_.HasCapability(SpvCapabilityKernel) &&
(opcode == SpvOpAtomicLoad || opcode == SpvOpAtomicExchange ||
opcode == SpvOpAtomicCompareExchange)) {
if (!_.IsFloatScalarType(result_type) &&
!_.IsIntScalarType(result_type)) {
const uint32_t result_type = inst->type_id();
// All current atomics only are scalar result
// Validate return type first so can just check if pointer type is same
// (if applicable)
if (HasReturnType(opcode)) {
if (HasOnlyFloatReturnType(opcode) &&
!_.IsFloatScalarType(result_type)) {
return _.diag(SPV_ERROR_INVALID_DATA, inst)
<< spvOpcodeString(opcode)
<< ": expected Result Type to be int or float scalar type";
}
} else if (opcode == SpvOpAtomicFlagTestAndSet) {
if (!_.IsBoolScalarType(result_type)) {
<< ": expected Result Type to be float scalar type";
} else if (HasOnlyIntReturnType(opcode) &&
!_.IsIntScalarType(result_type)) {
return _.diag(SPV_ERROR_INVALID_DATA, inst)
<< spvOpcodeString(opcode)
<< ": expected Result Type to be integer scalar type";
} else if (HasIntOrFloatReturnType(opcode) &&
!_.IsFloatScalarType(result_type) &&
!_.IsIntScalarType(result_type)) {
return _.diag(SPV_ERROR_INVALID_DATA, inst)
<< spvOpcodeString(opcode)
<< ": expected Result Type to be integer or float scalar type";
} else if (HasOnlyBoolReturnType(opcode) &&
!_.IsBoolScalarType(result_type)) {
return _.diag(SPV_ERROR_INVALID_DATA, inst)
<< spvOpcodeString(opcode)
<< ": expected Result Type to be bool scalar type";
}
} else if (opcode == SpvOpAtomicFlagClear || opcode == SpvOpAtomicStore) {
assert(result_type == 0);
} else {
if (_.IsFloatScalarType(result_type)) {
if (is_atomic_float_opcode) {
if (opcode == SpvOpAtomicFAddEXT) {
if ((_.GetBitWidth(result_type) == 32) &&
(!_.HasCapability(SpvCapabilityAtomicFloat32AddEXT))) {
return _.diag(SPV_ERROR_INVALID_DATA, inst)
<< spvOpcodeString(opcode)
<< ": float add atomics require the AtomicFloat32AddEXT "
"capability";
}
if ((_.GetBitWidth(result_type) == 64) &&
(!_.HasCapability(SpvCapabilityAtomicFloat64AddEXT))) {
return _.diag(SPV_ERROR_INVALID_DATA, inst)
<< spvOpcodeString(opcode)
<< ": float add atomics require the AtomicFloat64AddEXT "
"capability";
}
}
} else {
return _.diag(SPV_ERROR_INVALID_DATA, inst)
<< spvOpcodeString(opcode)
<< ": expected Result Type to be int scalar type";
}
} else if (_.IsIntScalarType(result_type) &&
opcode == SpvOpAtomicFAddEXT) {
return _.diag(SPV_ERROR_INVALID_DATA, inst)
<< spvOpcodeString(opcode)
<< ": expected Result Type to be float scalar type";
} else if (!_.IsFloatScalarType(result_type) &&
!_.IsIntScalarType(result_type)) {
switch (opcode) {
case SpvOpAtomicFAddEXT:
return _.diag(SPV_ERROR_INVALID_DATA, inst)
<< spvOpcodeString(opcode)
<< ": expected Result Type to be float scalar type";
case SpvOpAtomicIIncrement:
case SpvOpAtomicIDecrement:
case SpvOpAtomicIAdd:
case SpvOpAtomicISub:
case SpvOpAtomicSMin:
case SpvOpAtomicSMax:
case SpvOpAtomicUMin:
case SpvOpAtomicUMax:
return _.diag(SPV_ERROR_INVALID_DATA, inst)
<< spvOpcodeString(opcode)
<< ": expected Result Type to be integer scalar type";
default:
return _.diag(SPV_ERROR_INVALID_DATA, inst)
<< spvOpcodeString(opcode)
<< ": expected Result Type to be int or float scalar type";
}
}
}
uint32_t operand_index =
opcode == SpvOpAtomicFlagClear || opcode == SpvOpAtomicStore ? 0 : 2;
uint32_t operand_index = HasReturnType(opcode) ? 2 : 0;
const uint32_t pointer_type = _.GetOperandTypeId(inst, operand_index++);
uint32_t data_type = 0;
uint32_t storage_class = 0;
if (!_.GetPointerTypeInfo(pointer_type, &data_type, &storage_class)) {
@ -182,6 +196,7 @@ spv_result_t AtomicsPass(ValidationState_t& _, const Instruction* inst) {
// Then Shader rules
if (_.HasCapability(SpvCapabilityShader)) {
// Vulkan environment rule
if (spvIsVulkanEnv(_.context()->target_env)) {
if ((storage_class != SpvStorageClassUniform) &&
(storage_class != SpvStorageClassStorageBuffer) &&
@ -194,13 +209,30 @@ spv_result_t AtomicsPass(ValidationState_t& _, const Instruction* inst) {
"be: Uniform, Workgroup, Image, StorageBuffer, or "
"PhysicalStorageBuffer.";
}
} else if (storage_class == SpvStorageClassFunction) {
return _.diag(SPV_ERROR_INVALID_DATA, inst)
<< spvOpcodeString(opcode)
<< ": Function storage class forbidden when the Shader "
"capability is declared.";
}
if (opcode == SpvOpAtomicFAddEXT) {
// result type being float checked already
if ((_.GetBitWidth(result_type) == 32) &&
(!_.HasCapability(SpvCapabilityAtomicFloat32AddEXT))) {
return _.diag(SPV_ERROR_INVALID_DATA, inst)
<< spvOpcodeString(opcode)
<< ": float add atomics require the AtomicFloat32AddEXT "
"capability";
}
if ((_.GetBitWidth(result_type) == 64) &&
(!_.HasCapability(SpvCapabilityAtomicFloat64AddEXT))) {
return _.diag(SPV_ERROR_INVALID_DATA, inst)
<< spvOpcodeString(opcode)
<< ": float add atomics require the AtomicFloat64AddEXT "
"capability";
}
}
}
// And finally OpenCL environment rules
@ -224,27 +256,27 @@ spv_result_t AtomicsPass(ValidationState_t& _, const Instruction* inst) {
}
}
// If result and pointer type are different, need to do special check here
if (opcode == SpvOpAtomicFlagTestAndSet ||
opcode == SpvOpAtomicFlagClear) {
if (!_.IsIntScalarType(data_type) || _.GetBitWidth(data_type) != 32) {
return _.diag(SPV_ERROR_INVALID_DATA, inst)
<< spvOpcodeString(opcode)
<< ": expected Pointer to point to a value of 32-bit int type";
<< ": expected Pointer to point to a value of 32-bit integer "
"type";
}
} else if (opcode == SpvOpAtomicStore) {
if (!_.IsFloatScalarType(data_type) && !_.IsIntScalarType(data_type)) {
return _.diag(SPV_ERROR_INVALID_DATA, inst)
<< spvOpcodeString(opcode)
<< ": expected Pointer to be a pointer to int or float "
<< ": expected Pointer to be a pointer to integer or float "
<< "scalar type";
}
} else {
if (data_type != result_type) {
return _.diag(SPV_ERROR_INVALID_DATA, inst)
<< spvOpcodeString(opcode)
<< ": expected Pointer to point to a value of type Result "
"Type";
}
} else if (data_type != result_type) {
return _.diag(SPV_ERROR_INVALID_DATA, inst)
<< spvOpcodeString(opcode)
<< ": expected Pointer to point to a value of type Result "
"Type";
}
auto memory_scope = inst->GetOperandAs<const uint32_t>(operand_index++);

68
test/val/val_atomics_test.cpp
View File

@ -276,7 +276,7 @@ TEST_F(ValidateAtomics, AtomicAddIntVulkanWrongType1) {
ASSERT_EQ(SPV_ERROR_INVALID_DATA, ValidateInstructions());
EXPECT_THAT(getDiagnosticString(),
HasSubstr("AtomicIAdd: "
"expected Result Type to be int scalar type"));
"expected Result Type to be integer scalar type"));
}
TEST_F(ValidateAtomics, AtomicAddIntVulkanWrongType2) {
@ -666,9 +666,10 @@ TEST_F(ValidateAtomics, AtomicLoadWrongResultType) {
CompileSuccessfully(GenerateKernelCode(body));
ASSERT_EQ(SPV_ERROR_INVALID_DATA, ValidateInstructions());
EXPECT_THAT(getDiagnosticString(),
HasSubstr("AtomicLoad: "
"expected Result Type to be int or float scalar type"));
EXPECT_THAT(
getDiagnosticString(),
HasSubstr("AtomicLoad: "
"expected Result Type to be integer or float scalar type"));
}
TEST_F(ValidateAtomics, AtomicLoadWrongPointerType) {
@ -829,9 +830,9 @@ OpAtomicStore %f32vec4_var %device %relaxed %f32_1
ASSERT_EQ(SPV_ERROR_INVALID_DATA, ValidateInstructions());
EXPECT_THAT(
getDiagnosticString(),
HasSubstr("AtomicStore: "
"expected Pointer to be a pointer to int or float scalar "
"type"));
HasSubstr(
"AtomicStore: "
"expected Pointer to be a pointer to integer or float scalar type"));
}
TEST_F(ValidateAtomics, AtomicStoreWrongPointerStorageTypeForOpenCL) {
@ -937,9 +938,10 @@ OpStore %f32vec4_var %f32vec4_0000
CompileSuccessfully(GenerateKernelCode(body));
ASSERT_EQ(SPV_ERROR_INVALID_DATA, ValidateInstructions());
EXPECT_THAT(getDiagnosticString(),
HasSubstr("AtomicExchange: "
"expected Result Type to be int or float scalar type"));
EXPECT_THAT(
getDiagnosticString(),
HasSubstr("AtomicExchange: "
"expected Result Type to be integer or float scalar type"));
}
TEST_F(ValidateAtomics, AtomicExchangeWrongPointerType) {
@ -1035,10 +1037,8 @@ OpAtomicStore %u32_var %device %relaxed %u32_1
TEST_F(ValidateAtomics, AtomicCompareExchangeKernelSuccess) {
const std::string body = R"(
OpAtomicStore %f32_var %device %relaxed %f32_1
%val2 = OpAtomicCompareExchange %f32 %f32_var %device %relaxed %relaxed %f32_0 %f32_1
OpAtomicStore %u32_var %device %relaxed %u32_1
%val4 = OpAtomicCompareExchange %u32 %u32_var %device %relaxed %relaxed %u32_0 %u32_0
%val2 = OpAtomicCompareExchange %u32 %u32_var %device %relaxed %relaxed %u32_0 %u32_0
)";
CompileSuccessfully(GenerateKernelCode(body));
@ -1055,7 +1055,7 @@ OpAtomicStore %f32_var %device %relaxed %f32_1
ASSERT_EQ(SPV_ERROR_INVALID_DATA, ValidateInstructions());
EXPECT_THAT(getDiagnosticString(),
HasSubstr("AtomicCompareExchange: "
"expected Result Type to be int scalar type"));
"expected Result Type to be integer scalar type"));
}
TEST_F(ValidateAtomics, AtomicCompareExchangeWrongResultType) {
@ -1068,7 +1068,7 @@ OpStore %f32vec4_var %f32vec4_0000
ASSERT_EQ(SPV_ERROR_INVALID_DATA, ValidateInstructions());
EXPECT_THAT(getDiagnosticString(),
HasSubstr("AtomicCompareExchange: "
"expected Result Type to be int or float scalar type"));
"expected Result Type to be integer scalar type"));
}
TEST_F(ValidateAtomics, AtomicCompareExchangeWrongPointerType) {
@ -1086,7 +1086,7 @@ TEST_F(ValidateAtomics, AtomicCompareExchangeWrongPointerType) {
TEST_F(ValidateAtomics, AtomicCompareExchangeWrongPointerDataType) {
const std::string body = R"(
OpStore %f32vec4_var %f32vec4_0000
%val2 = OpAtomicCompareExchange %f32 %f32vec4_var %device %relaxed %relaxed %f32_0 %f32_1
%val2 = OpAtomicCompareExchange %u32 %f32vec4_var %device %relaxed %relaxed %u32_0 %u32_0
)";
CompileSuccessfully(GenerateKernelCode(body));
@ -1099,11 +1099,11 @@ OpStore %f32vec4_var %f32vec4_0000
TEST_F(ValidateAtomics, AtomicCompareExchangeWrongScopeType) {
const std::string body = R"(
OpAtomicStore %f32_var %device %relaxed %f32_1
%val2 = OpAtomicCompareExchange %f32 %f32_var %f32_1 %relaxed %relaxed %f32_0 %f32_0
OpAtomicStore %u64_var %device %relaxed %u64_1
%val2 = OpAtomicCompareExchange %u64 %u64_var %u64_1 %relaxed %relaxed %u32_0 %u32_0
)";
CompileSuccessfully(GenerateKernelCode(body));
CompileSuccessfully(GenerateKernelCode(body, "OpCapability Int64Atomics\n"));
ASSERT_EQ(SPV_ERROR_INVALID_DATA, ValidateInstructions());
EXPECT_THAT(getDiagnosticString(),
HasSubstr("AtomicCompareExchange: expected scope to be a 32-bit "
@ -1112,8 +1112,8 @@ OpAtomicStore %f32_var %device %relaxed %f32_1
TEST_F(ValidateAtomics, AtomicCompareExchangeWrongMemorySemanticsType1) {
const std::string body = R"(
OpAtomicStore %f32_var %device %relaxed %f32_1
%val2 = OpAtomicCompareExchange %f32 %f32_var %device %f32_1 %relaxed %f32_0 %f32_0
OpAtomicStore %u32_var %device %relaxed %u32_1
%val2 = OpAtomicCompareExchange %u32 %u32_var %device %f32_1 %relaxed %u32_0 %u32_0
)";
CompileSuccessfully(GenerateKernelCode(body));
@ -1125,8 +1125,8 @@ OpAtomicStore %f32_var %device %relaxed %f32_1
TEST_F(ValidateAtomics, AtomicCompareExchangeWrongMemorySemanticsType2) {
const std::string body = R"(
OpAtomicStore %f32_var %device %relaxed %f32_1
%val2 = OpAtomicCompareExchange %f32 %f32_var %device %relaxed %f32_1 %f32_0 %f32_0
OpAtomicStore %u32_var %device %relaxed %u32_1
%val2 = OpAtomicCompareExchange %u32 %u32_var %device %relaxed %f32_1 %u32_0 %u32_0
)";
CompileSuccessfully(GenerateKernelCode(body));
@ -1138,8 +1138,8 @@ OpAtomicStore %f32_var %device %relaxed %f32_1
TEST_F(ValidateAtomics, AtomicCompareExchangeUnequalRelease) {
const std::string body = R"(
OpAtomicStore %f32_var %device %relaxed %f32_1
%val2 = OpAtomicCompareExchange %f32 %f32_var %device %relaxed %release %f32_0 %f32_0
OpAtomicStore %u32_var %device %relaxed %u32_1
%val2 = OpAtomicCompareExchange %u32 %u32_var %device %relaxed %release %u32_0 %u32_0
)";
CompileSuccessfully(GenerateKernelCode(body));
@ -1151,8 +1151,8 @@ OpAtomicStore %f32_var %device %relaxed %f32_1
TEST_F(ValidateAtomics, AtomicCompareExchangeWrongValueType) {
const std::string body = R"(
OpAtomicStore %f32_var %device %relaxed %f32_1
%val2 = OpAtomicCompareExchange %f32 %f32_var %device %relaxed %relaxed %u32_0 %f32_1
OpAtomicStore %u32_var %device %relaxed %u32_1
%val2 = OpAtomicCompareExchange %u32 %u32_var %device %relaxed %relaxed %f32_1 %u32_0
)";
CompileSuccessfully(GenerateKernelCode(body));
@ -1164,8 +1164,8 @@ OpAtomicStore %f32_var %device %relaxed %f32_1
TEST_F(ValidateAtomics, AtomicCompareExchangeWrongComparatorType) {
const std::string body = R"(
OpAtomicStore %f32_var %device %relaxed %f32_1
%val2 = OpAtomicCompareExchange %f32 %f32_var %device %relaxed %relaxed %f32_0 %u32_1
OpAtomicStore %u32_var %device %relaxed %u32_1
%val2 = OpAtomicCompareExchange %u32 %u32_var %device %relaxed %relaxed %u32_0 %f32_0
)";
CompileSuccessfully(GenerateKernelCode(body));
@ -1195,7 +1195,7 @@ OpAtomicStore %f32_var %device %relaxed %f32_1
ASSERT_EQ(SPV_ERROR_INVALID_DATA, ValidateInstructions());
EXPECT_THAT(getDiagnosticString(),
HasSubstr("AtomicCompareExchangeWeak: "
"expected Result Type to be int scalar type"));
"expected Result Type to be integer scalar type"));
}
TEST_F(ValidateAtomics, AtomicCompareExchangeVulkanInvocationSemanticsEqual) {
@ -1294,7 +1294,7 @@ TEST_F(ValidateAtomics, AtomicFlagTestAndSetNotIntPointer) {
EXPECT_THAT(
getDiagnosticString(),
HasSubstr("AtomicFlagTestAndSet: "
"expected Pointer to point to a value of 32-bit int type"));
"expected Pointer to point to a value of 32-bit integer type"));
}
TEST_F(ValidateAtomics, AtomicFlagTestAndSetNotInt32Pointer) {
@ -1307,7 +1307,7 @@ TEST_F(ValidateAtomics, AtomicFlagTestAndSetNotInt32Pointer) {
EXPECT_THAT(
getDiagnosticString(),
HasSubstr("AtomicFlagTestAndSet: "
"expected Pointer to point to a value of 32-bit int type"));
"expected Pointer to point to a value of 32-bit integer type"));
}
TEST_F(ValidateAtomics, AtomicFlagTestAndSetWrongScopeType) {
@ -1368,7 +1368,7 @@ OpAtomicFlagClear %f32_var %device %relaxed
EXPECT_THAT(
getDiagnosticString(),
HasSubstr("AtomicFlagClear: "
"expected Pointer to point to a value of 32-bit int type"));
"expected Pointer to point to a value of 32-bit integer type"));
}
TEST_F(ValidateAtomics, AtomicFlagClearNotInt32Pointer) {
@ -1381,7 +1381,7 @@ OpAtomicFlagClear %u64_var %device %relaxed
EXPECT_THAT(
getDiagnosticString(),
HasSubstr("AtomicFlagClear: "
"expected Pointer to point to a value of 32-bit int type"));
"expected Pointer to point to a value of 32-bit integer type"));
}
TEST_F(ValidateAtomics, AtomicFlagClearWrongScopeType) {