spirv-val: Clean up VariablePointers logic (#4755)

This commit is contained in:
sfricke-samsung 2022-03-24 11:02:29 -05:00 committed by GitHub
parent cab0b7715a
commit 90728d2dff
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
7 changed files with 26 additions and 47 deletions

View File

@ -55,8 +55,7 @@ spv_result_t ValidatePhi(ValidationState_t& _, const Instruction* inst) {
}
if (_.IsPointerType(inst->type_id()) &&
_.addressing_model() == SpvAddressingModelLogical) {
if (!_.features().variable_pointers &&
!_.features().variable_pointers_storage_buffer) {
if (!_.features().variable_pointers) {
return _.diag(SPV_ERROR_INVALID_DATA, inst)
<< "Using pointers with OpPhi requires capability "
<< "VariablePointers or VariablePointersStorageBuffer";
@ -249,13 +248,9 @@ spv_result_t ValidateReturnValue(ValidationState_t& _,
<< _.getIdName(value->type_id()) << "' is missing or void.";
}
const bool uses_variable_pointer =
_.features().variable_pointers ||
_.features().variable_pointers_storage_buffer;
if (_.addressing_model() == SpvAddressingModelLogical &&
SpvOpTypePointer == value_type->opcode() && !uses_variable_pointer &&
!_.options()->relax_logical_pointer) {
SpvOpTypePointer == value_type->opcode() &&
!_.features().variable_pointers && !_.options()->relax_logical_pointer) {
return _.diag(SPV_ERROR_INVALID_ID, inst)
<< "OpReturnValue value's type <id> '"
<< _.getIdName(value->type_id())

View File

@ -300,7 +300,7 @@ spv_result_t ValidateFunctionCall(ValidationState_t& _,
// These are always allowed.
break;
case SpvStorageClassStorageBuffer:
if (!_.features().variable_pointers_storage_buffer) {
if (!_.features().variable_pointers) {
return _.diag(SPV_ERROR_INVALID_ID, inst)
<< "StorageBuffer pointer operand "
<< _.getIdName(argument_id)
@ -316,11 +316,10 @@ spv_result_t ValidateFunctionCall(ValidationState_t& _,
// Validate memory object declaration requirements.
if (argument->opcode() != SpvOpVariable &&
argument->opcode() != SpvOpFunctionParameter) {
const bool ssbo_vptr =
_.features().variable_pointers_storage_buffer &&
sc == SpvStorageClassStorageBuffer;
const bool wg_vptr =
_.features().variable_pointers && sc == SpvStorageClassWorkgroup;
const bool ssbo_vptr = _.features().variable_pointers &&
sc == SpvStorageClassStorageBuffer;
const bool wg_vptr = _.HasCapability(SpvCapabilityVariablePointers) &&
sc == SpvStorageClassWorkgroup;
const bool uc_ptr = sc == SpvStorageClassUniformConstant;
if (!ssbo_vptr && !wg_vptr && !uc_ptr) {
return _.diag(SPV_ERROR_INVALID_ID, inst)

View File

@ -163,8 +163,7 @@ spv_result_t LogicalsPass(ValidationState_t& _, const Instruction* inst) {
switch (type_opcode) {
case SpvOpTypePointer: {
if (_.addressing_model() == SpvAddressingModelLogical &&
!_.features().variable_pointers &&
!_.features().variable_pointers_storage_buffer)
!_.features().variable_pointers)
return _.diag(SPV_ERROR_INVALID_DATA, inst)
<< "Using pointers with OpSelect requires capability "
<< "VariablePointers or VariablePointersStorageBuffer";

View File

@ -870,17 +870,14 @@ spv_result_t ValidateLoad(ValidationState_t& _, const Instruction* inst) {
<< "' is not defined.";
}
const bool uses_variable_pointers =
_.features().variable_pointers ||
_.features().variable_pointers_storage_buffer;
const auto pointer_index = 2;
const auto pointer_id = inst->GetOperandAs<uint32_t>(pointer_index);
const auto pointer = _.FindDef(pointer_id);
if (!pointer ||
((_.addressing_model() == SpvAddressingModelLogical) &&
((!uses_variable_pointers &&
((!_.features().variable_pointers &&
!spvOpcodeReturnsLogicalPointer(pointer->opcode())) ||
(uses_variable_pointers &&
(_.features().variable_pointers &&
!spvOpcodeReturnsLogicalVariablePointer(pointer->opcode()))))) {
return _.diag(SPV_ERROR_INVALID_ID, inst)
<< "OpLoad Pointer <id> '" << _.getIdName(pointer_id)
@ -926,17 +923,14 @@ spv_result_t ValidateLoad(ValidationState_t& _, const Instruction* inst) {
}
spv_result_t ValidateStore(ValidationState_t& _, const Instruction* inst) {
const bool uses_variable_pointer =
_.features().variable_pointers ||
_.features().variable_pointers_storage_buffer;
const auto pointer_index = 0;
const auto pointer_id = inst->GetOperandAs<uint32_t>(pointer_index);
const auto pointer = _.FindDef(pointer_id);
if (!pointer ||
(_.addressing_model() == SpvAddressingModelLogical &&
((!uses_variable_pointer &&
((!_.features().variable_pointers &&
!spvOpcodeReturnsLogicalPointer(pointer->opcode())) ||
(uses_variable_pointer &&
(_.features().variable_pointers &&
!spvOpcodeReturnsLogicalVariablePointer(pointer->opcode()))))) {
return _.diag(SPV_ERROR_INVALID_ID, inst)
<< "OpStore Pointer <id> '" << _.getIdName(pointer_id)
@ -1362,8 +1356,7 @@ spv_result_t ValidateAccessChain(ValidationState_t& _,
spv_result_t ValidatePtrAccessChain(ValidationState_t& _,
const Instruction* inst) {
if (_.addressing_model() == SpvAddressingModelLogical) {
if (!_.features().variable_pointers &&
!_.features().variable_pointers_storage_buffer) {
if (!_.features().variable_pointers) {
return _.diag(SPV_ERROR_INVALID_DATA, inst)
<< "Generating variable pointers requires capability "
<< "VariablePointers or VariablePointersStorageBuffer";
@ -1481,18 +1474,15 @@ spv_result_t ValidateCooperativeMatrixLoadStoreNV(ValidationState_t& _,
}
}
const bool uses_variable_pointers =
_.features().variable_pointers ||
_.features().variable_pointers_storage_buffer;
const auto pointer_index =
(inst->opcode() == SpvOpCooperativeMatrixLoadNV) ? 2u : 0u;
const auto pointer_id = inst->GetOperandAs<uint32_t>(pointer_index);
const auto pointer = _.FindDef(pointer_id);
if (!pointer ||
((_.addressing_model() == SpvAddressingModelLogical) &&
((!uses_variable_pointers &&
((!_.features().variable_pointers &&
!spvOpcodeReturnsLogicalPointer(pointer->opcode())) ||
(uses_variable_pointers &&
(_.features().variable_pointers &&
!spvOpcodeReturnsLogicalVariablePointer(pointer->opcode()))))) {
return _.diag(SPV_ERROR_INVALID_ID, inst)
<< opname << " Pointer <id> '" << _.getIdName(pointer_id)
@ -1564,10 +1554,10 @@ spv_result_t ValidateCooperativeMatrixLoadStoreNV(ValidationState_t& _,
spv_result_t ValidatePtrComparison(ValidationState_t& _,
const Instruction* inst) {
if (_.addressing_model() == SpvAddressingModelLogical &&
!_.features().variable_pointers_storage_buffer) {
!_.features().variable_pointers) {
return _.diag(SPV_ERROR_INVALID_ID, inst)
<< "Instruction cannot be used without a variable pointers "
"capability";
<< "Instruction cannot for logical addressing model be used without "
"a variable pointers capability";
}
const auto result_type = _.FindDef(inst->type_id());
@ -1602,7 +1592,8 @@ spv_result_t ValidatePtrComparison(ValidationState_t& _,
<< "Invalid pointer storage class";
}
if (sc == SpvStorageClassWorkgroup && !_.features().variable_pointers) {
if (sc == SpvStorageClassWorkgroup &&
!_.HasCapability(SpvCapabilityVariablePointers)) {
return _.diag(SPV_ERROR_INVALID_ID, inst)
<< "Workgroup storage class pointer requires VariablePointers "
"capability to be specified";

View File

@ -392,11 +392,8 @@ void ValidationState_t::RegisterCapability(SpvCapability cap) {
features_.free_fp_rounding_mode = true;
break;
case SpvCapabilityVariablePointers:
features_.variable_pointers = true;
features_.variable_pointers_storage_buffer = true;
break;
case SpvCapabilityVariablePointersStorageBuffer:
features_.variable_pointers_storage_buffer = true;
features_.variable_pointers = true;
break;
default:
// TODO(dneto): For now don't validate SPV_NV_ray_tracing, which uses

View File

@ -70,11 +70,9 @@ class ValidationState_t {
// and its values to be used without
// requiring any capability
// Allow functionalities enabled by VariablePointers capability.
// Allow functionalities enabled by VariablePointers or
// VariablePointersStorageBuffer capability.
bool variable_pointers = false;
// Allow functionalities enabled by VariablePointersStorageBuffer
// capability.
bool variable_pointers_storage_buffer = false;
// Permit group oerations Reduce, InclusiveScan, ExclusiveScan
bool group_ops_reduce_and_scans = false;

View File

@ -3252,8 +3252,8 @@ OpFunctionEnd
EXPECT_EQ(SPV_ERROR_INVALID_ID,
ValidateInstructions(SPV_ENV_UNIVERSAL_1_4));
EXPECT_THAT(getDiagnosticString(),
HasSubstr("Instruction cannot be used without a variable "
"pointers capability"));
HasSubstr("Instruction cannot for logical addressing model be "
"used without a variable pointers capability"));
}
}