spirv-val: Add remaining OpSpecConstantOp (#6596)
My personal final PR for
https://github.com/KhronosGroup/SPIRV-Tools/issues/6564
There is still the access chains left, but those are going to be ugly,
and since its only a `Kernel` feature, I don't have the bandwidth to
look into right now. (But did left some test for the person who does in
the future)
diff --git a/source/val/validate_composites.cpp b/source/val/validate_composites.cpp
index c9aadca..98c1343 100644
--- a/source/val/validate_composites.cpp
+++ b/source/val/validate_composites.cpp
@@ -17,6 +17,7 @@
// Validates correctness of composite SPIR-V instructions.
#include <climits>
+#include <cstdint>
#include "source/opcode.h"
#include "source/spirv_target_env.h"
@@ -36,14 +37,11 @@
// deep).
spv_result_t GetExtractInsertValueType(ValidationState_t& _,
const Instruction* inst,
- uint32_t* member_type) {
- const spv::Op opcode = inst->opcode();
- assert(opcode == spv::Op::OpCompositeExtract ||
- opcode == spv::Op::OpCompositeInsert);
- uint32_t word_index = opcode == spv::Op::OpCompositeExtract ? 4 : 5;
- const uint32_t num_words = static_cast<uint32_t>(inst->words().size());
- const uint32_t composite_id_index = word_index - 1;
- const uint32_t num_indices = num_words - word_index;
+ uint32_t* member_type,
+ uint32_t composite_id_index) {
+ const uint32_t num_operands = static_cast<uint32_t>(inst->operands().size());
+ const uint32_t first_literal_index = composite_id_index + 1;
+ const uint32_t num_indices = num_operands - first_literal_index;
const uint32_t kCompositeExtractInsertMaxNumIndices = 255;
if (num_indices == 0) {
@@ -53,19 +51,21 @@
} else if (num_indices > kCompositeExtractInsertMaxNumIndices) {
return _.diag(SPV_ERROR_INVALID_DATA, inst)
- << "The number of indexes in Op" << spvOpcodeString(opcode)
+ << "The number of indexes in Op" << spvOpcodeString(inst->opcode())
<< " may not exceed " << kCompositeExtractInsertMaxNumIndices
<< ". Found " << num_indices << " indexes.";
}
- *member_type = _.GetTypeId(inst->word(composite_id_index));
+ *member_type = _.GetOperandTypeId(inst, composite_id_index);
if (*member_type == 0) {
return _.diag(SPV_ERROR_INVALID_DATA, inst)
<< "Expected Composite to be an object of composite type";
}
- for (; word_index < num_words; ++word_index) {
- const uint32_t component_index = inst->word(word_index);
+ for (uint32_t operand_index = first_literal_index;
+ operand_index < num_operands; ++operand_index) {
+ const uint32_t component_index =
+ inst->GetOperandAs<uint32_t>(operand_index);
const Instruction* const type_inst = _.FindDef(*member_type);
assert(type_inst);
switch (type_inst->opcode()) {
@@ -471,9 +471,12 @@
}
spv_result_t ValidateCompositeExtract(ValidationState_t& _,
- const Instruction* inst) {
+ const Instruction* inst,
+ uint32_t operand_index = 2) {
uint32_t member_type = 0;
- if (spv_result_t error = GetExtractInsertValueType(_, inst, &member_type)) {
+
+ if (spv_result_t error =
+ GetExtractInsertValueType(_, inst, &member_type, operand_index)) {
return error;
}
@@ -496,9 +499,10 @@
}
spv_result_t ValidateCompositeInsert(ValidationState_t& _,
- const Instruction* inst) {
- const uint32_t object_type = _.GetOperandTypeId(inst, 2);
- const uint32_t composite_type = _.GetOperandTypeId(inst, 3);
+ const Instruction* inst,
+ uint32_t operand_index = 2) {
+ const uint32_t object_type = _.GetOperandTypeId(inst, operand_index);
+ const uint32_t composite_type = _.GetOperandTypeId(inst, operand_index + 1);
const uint32_t result_type = inst->type_id();
if (result_type != composite_type) {
return _.diag(SPV_ERROR_INVALID_DATA, inst)
@@ -508,7 +512,8 @@
}
uint32_t member_type = 0;
- if (spv_result_t error = GetExtractInsertValueType(_, inst, &member_type)) {
+ if (spv_result_t error =
+ GetExtractInsertValueType(_, inst, &member_type, operand_index + 1)) {
return error;
}
@@ -589,58 +594,58 @@
}
spv_result_t ValidateVectorShuffle(ValidationState_t& _,
- const Instruction* inst) {
- auto resultType = _.FindDef(inst->type_id());
- if (!_.IsVectorType(resultType->id())) {
+ const Instruction* inst,
+ uint32_t operand_index = 2) {
+ auto result_type = _.FindDef(inst->type_id());
+ if (!_.IsVectorType(result_type->id())) {
return _.diag(SPV_ERROR_INVALID_ID, inst)
<< "The Result Type of OpVectorShuffle must be"
<< " a vector type. Found Op"
- << spvOpcodeString(resultType->opcode()) << ".";
+ << spvOpcodeString(result_type->opcode()) << ".";
}
// The number of components in Result Type must be the same as the number of
// Component operands.
- auto componentCount = inst->operands().size() - 4;
- auto resultVectorDimension = _.GetDimension(resultType->id());
- if (resultVectorDimension > 0 && componentCount != resultVectorDimension) {
+ uint32_t first_literal_index = operand_index + 2;
+ uint32_t component_count =
+ static_cast<uint32_t>(inst->operands().size()) - first_literal_index;
+ auto result_vec_dimension = _.GetDimension(result_type->id());
+ if (result_vec_dimension > 0 && component_count != result_vec_dimension) {
return _.diag(SPV_ERROR_INVALID_ID, inst)
<< "OpVectorShuffle component literals count does not match "
"Result Type <id> "
- << _.getIdName(resultType->id()) << "s vector component count.";
+ << _.getIdName(result_type->id()) << "s vector component count.";
}
// Vector 1 and Vector 2 must both have vector types, with the same Component
// Type as Result Type.
- auto vector1Object = _.FindDef(inst->GetOperandAs<uint32_t>(2));
- auto vector1Type = _.FindDef(vector1Object->type_id());
- auto vector2Object = _.FindDef(inst->GetOperandAs<uint32_t>(3));
- auto vector2Type = _.FindDef(vector2Object->type_id());
- if (!_.IsVectorType(vector1Type->id())) {
+ auto vec1_type = _.FindDef(_.GetOperandTypeId(inst, operand_index));
+ auto vec2_type = _.FindDef(_.GetOperandTypeId(inst, operand_index + 1));
+ if (!_.IsVectorType(vec1_type->id())) {
return _.diag(SPV_ERROR_INVALID_ID, inst)
<< "The type of Vector 1 must be a vector type.";
}
- if (!_.IsVectorType(vector2Type->id())) {
+ if (!_.IsVectorType(vec2_type->id())) {
return _.diag(SPV_ERROR_INVALID_ID, inst)
<< "The type of Vector 2 must be a vector type.";
}
- auto resultComponentType = resultType->GetOperandAs<uint32_t>(1);
- if (vector1Type->GetOperandAs<uint32_t>(1) != resultComponentType) {
+ uint32_t result_component_type = result_type->GetOperandAs<uint32_t>(1);
+ if (vec1_type->GetOperandAs<uint32_t>(1) != result_component_type) {
return _.diag(SPV_ERROR_INVALID_ID, inst)
<< "The Component Type of Vector 1 must be the same as ResultType.";
}
- if (vector2Type->GetOperandAs<uint32_t>(1) != resultComponentType) {
+ if (vec2_type->GetOperandAs<uint32_t>(1) != result_component_type) {
return _.diag(SPV_ERROR_INVALID_ID, inst)
<< "The Component Type of Vector 2 must be the same as ResultType.";
}
// All Component literals must either be FFFFFFFF or in [0, N - 1].
- auto vector1ComponentCount = vector1Type->GetOperandAs<uint32_t>(2);
- auto vector2ComponentCount = vector2Type->GetOperandAs<uint32_t>(2);
- auto N = vector1ComponentCount + vector2ComponentCount;
- auto firstLiteralIndex = 4;
- for (size_t i = firstLiteralIndex; i < inst->operands().size(); ++i) {
- auto literal = inst->GetOperandAs<uint32_t>(i);
+ uint32_t vec1_component_count = vec1_type->GetOperandAs<uint32_t>(2);
+ uint32_t vec2_component_count = vec2_type->GetOperandAs<uint32_t>(2);
+ uint32_t N = vec1_component_count + vec2_component_count;
+ for (size_t i = first_literal_index; i < inst->operands().size(); ++i) {
+ uint32_t literal = inst->GetOperandAs<uint32_t>(i);
if (literal != 0xFFFFFFFF && literal >= N) {
return _.diag(SPV_ERROR_INVALID_ID, inst)
<< "Component index " << literal << " is out of bounds for "
@@ -1168,6 +1173,20 @@
return ValidateCompositeExtractCoopMatQCOM(_, inst);
case spv::Op::OpExtractSubArrayQCOM:
return ValidateExtractSubArrayQCOM(_, inst);
+
+ case spv::Op::OpSpecConstantOp: {
+ switch (inst->GetOperandAs<spv::Op>(2u)) {
+ case spv::Op::OpVectorShuffle:
+ return ValidateVectorShuffle(_, inst, 3);
+ case spv::Op::OpCompositeExtract:
+ return ValidateCompositeExtract(_, inst, 3);
+ case spv::Op::OpCompositeInsert:
+ return ValidateCompositeInsert(_, inst, 3);
+ default:
+ break;
+ }
+ }
+
default:
break;
}
diff --git a/source/val/validate_memory.cpp b/source/val/validate_memory.cpp
index 4ade235..47d1a3c 100644
--- a/source/val/validate_memory.cpp
+++ b/source/val/validate_memory.cpp
@@ -3535,13 +3535,14 @@
return ValidatePtrComparison(_, inst);
case spv::Op::OpImageTexelPointer:
case spv::Op::OpGenericPtrMemSemantics:
-
+ break; // no validation currently
case spv::Op::OpSpecConstantOp: {
switch (inst->GetOperandAs<spv::Op>(2u)) {
case spv::Op::OpCooperativeMatrixLengthKHR:
return ValidateCooperativeMatrixLength(_, inst, true, 3);
case spv::Op::OpCooperativeMatrixLengthNV:
return ValidateCooperativeMatrixLength(_, inst, false, 3);
+ // TODO - Add AccesChains
default:
break;
}
diff --git a/test/val/val_constants_test.cpp b/test/val/val_constants_test.cpp
index 71d17ec..b50266d 100644
--- a/test/val/val_constants_test.cpp
+++ b/test/val/val_constants_test.cpp
@@ -772,12 +772,16 @@
HasSubstr("must be OpTypeCooperativeMatrixKHR"));
}
+// Some check use SPV_ERROR_INVALID_DATA vs SPV_ERROR_INVALID_ID
#define BAD_KERNEL_OPERANDS(STR, ERR) \
{ \
SPV_ENV_UNIVERSAL_1_0, kKernelPreamble kBasicTypes STR, false, ERR, \
SPV_ERROR_INVALID_DATA \
}
+#define BAD_KERNEL_OPERANDS_ID(STR, ERR) \
+ { SPV_ENV_UNIVERSAL_1_0, kKernelPreamble kBasicTypes STR, false, ERR, }
+
// 2 of each, first has bad return type, second has bad operand
INSTANTIATE_TEST_SUITE_P(
BadOperandsKernel, ValidateConstantOp,
@@ -1079,7 +1083,57 @@
"float vector or scalar type"),
BAD_KERNEL_OPERANDS("%v = OpSpecConstantOp %uint Bitcast %true",
"Expected input to be a pointer or int or float "
- "vector or scalar")}));
+ "vector or scalar"),
+
+ BAD_KERNEL_OPERANDS_ID(
+ "%v = OpSpecConstantOp %float VectorShuffle %uint2_0 %uint2_0 1 3",
+ "The Result Type of OpVectorShuffle must be a vector type"),
+ BAD_KERNEL_OPERANDS_ID(
+ "%v = OpSpecConstantOp %uint2 VectorShuffle %uint2_0 %uint_0 1 3",
+ "The type of Vector 2 must be a vector type"),
+ BAD_KERNEL_OPERANDS(
+ "%v = OpSpecConstantOp %float CompositeExtract %uint2_0 1",
+ "Result type (OpTypeFloat) does not match the type that results "
+ "from indexing into the composite (OpTypeInt)"),
+ BAD_KERNEL_OPERANDS(
+ "%v = OpSpecConstantOp %uint CompositeExtract %uint_0 1",
+ "Reached non-composite type while indexes still remain to be "
+ "traversed"),
+ BAD_KERNEL_OPERANDS(
+ "%v = OpSpecConstantOp %float CompositeInsert %uint_0 %uint2_0 1",
+ "The Result Type must be the same as Composite type in "
+ "OpSpecConstantOp yielding Result Id 5"),
+ BAD_KERNEL_OPERANDS(
+ "%v = OpSpecConstantOp %uint2 CompositeInsert %uint_0 %uint_0 1",
+ "The Result Type must be the same as Composite type in "
+ "OpSpecConstantOp yielding Result Id 4"),
+
+ // TODO - Still need to add access chains
+ //
+ // BAD_KERNEL_OPERANDS("%v = OpSpecConstantOp %uint AccessChain %null",
+ // "AccessChain"),
+ // BAD_KERNEL_OPERANDS("%v = OpSpecConstantOp %_ptr_uint AccessChain
+ // %null %float_0",
+ // "AccessChain"),
+ // BAD_KERNEL_OPERANDS("%v = OpSpecConstantOp %uint InBoundsAccessChain
+ // %null",
+ // "InBoundsAccessChain"),
+ // BAD_KERNEL_OPERANDS("%v = OpSpecConstantOp %_ptr_uint
+ // InBoundsAccessChain %null %float_0",
+ // "InBoundsAccessChain"),
+ // BAD_KERNEL_OPERANDS("%v = OpSpecConstantOp %uint PtrAccessChain %null
+ // %uint_0",
+ // "PtrAccessChain"),
+ // BAD_KERNEL_OPERANDS("%v = OpSpecConstantOp %_ptr_uint PtrAccessChain
+ // %float_0 %float_0",
+ // "PtrAccessChain"),
+ // BAD_KERNEL_OPERANDS("%v = OpSpecConstantOp %uint
+ // InBoundsPtrAccessChain %null %uint_0",
+ // "InBoundsPtrAccessChain"),
+ // BAD_KERNEL_OPERANDS("%v = OpSpecConstantOp %_ptr_uint
+ // InBoundsPtrAccessChain %float_0 %float_0",
+ // "InBoundsPtrAccessChain"),
+ }));
} // namespace
} // namespace val
