spirv-val: Clean up VariablePointers logic (#4755)
diff --git a/source/val/validate_cfg.cpp b/source/val/validate_cfg.cpp
index 88abd75..dd605d2 100644
--- a/source/val/validate_cfg.cpp
+++ b/source/val/validate_cfg.cpp
@@ -55,8 +55,7 @@
}
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 @@
<< _.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())
diff --git a/source/val/validate_function.cpp b/source/val/validate_function.cpp
index 656893f..2a5fed8 100644
--- a/source/val/validate_function.cpp
+++ b/source/val/validate_function.cpp
@@ -300,7 +300,7 @@
// 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 @@
// 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)
diff --git a/source/val/validate_logicals.cpp b/source/val/validate_logicals.cpp
index bb35f55..5307988 100644
--- a/source/val/validate_logicals.cpp
+++ b/source/val/validate_logicals.cpp
@@ -163,8 +163,7 @@
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";
diff --git a/source/val/validate_memory.cpp b/source/val/validate_memory.cpp
index 0b23126..af9da67 100644
--- a/source/val/validate_memory.cpp
+++ b/source/val/validate_memory.cpp
@@ -870,17 +870,14 @@
<< "' 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 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 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 @@
}
}
- 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 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 @@
<< "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";
diff --git a/source/val/validation_state.cpp b/source/val/validation_state.cpp
index 6483335..ea626b1 100644
--- a/source/val/validation_state.cpp
+++ b/source/val/validation_state.cpp
@@ -392,11 +392,8 @@
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
diff --git a/source/val/validation_state.h b/source/val/validation_state.h
index 89834a0..4888840 100644
--- a/source/val/validation_state.h
+++ b/source/val/validation_state.h
@@ -70,11 +70,9 @@
// 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;
diff --git a/test/val/val_memory_test.cpp b/test/val/val_memory_test.cpp
index f4369c6..5fb43f7 100644
--- a/test/val/val_memory_test.cpp
+++ b/test/val/val_memory_test.cpp
@@ -3252,8 +3252,8 @@
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"));
}
}
