Add option to relax validation of store types.

There are a number of users of spriv-opt that are hitting errors
because of stores with different types.  In general, this is wrong, but,
in these cases, the types are the exact same except for decorations.

The options is "--relax-store-struct", and it can be used with the
validator or the optimizer.

We assume that if layout information is missing it is consistent.  For
example if one struct has a offset of one of its members, and the other
one does not, we will still consider them as being layout compatible.
The problem will be if both struct has and offset decoration for
corresponding members, and the offset are different.
This commit is contained in:
Steven Perron 2017-10-24 15:13:13 -04:00 committed by David Neto
parent 6724c27251
commit 716138ee14
9 changed files with 522 additions and 126 deletions

View File

@ -420,6 +420,19 @@ void spvValidatorOptionsSetUniversalLimit(spv_validator_options options,
spv_validator_limit limit_type,
uint32_t limit);
// Record whether or not the validator should relax the rules on types for
// stores to structs. When relaxed, it will allow a type mismatch as long as
// the types are structs with the same layout. Two structs have the same layout
// if
//
// 1) the members of the structs are either the same type or are structs with
// same layout, and
//
// 2) the decorations that affect the memory layout are identical for both
// types. Other decorations are not relevant.
void spvValidatorOptionsSetRelaxStoreStruct(spv_validator_options options,
bool val);
// Encodes the given SPIR-V assembly text to its binary representation. The
// length parameter specifies the number of bytes for text. Encoded binary will
// be stored into *binary. Any error will be written into *diagnostic if
@ -432,9 +445,11 @@ spv_result_t spvTextToBinary(const spv_const_context context, const char* text,
// Encodes the given SPIR-V assembly text to its binary representation. Same as
// spvTextToBinary but with options. The options parameter is a bit field of
// spv_text_to_binary_options_t.
spv_result_t spvTextToBinaryWithOptions(
const spv_const_context context, const char* text, const size_t length,
const uint32_t options, spv_binary* binary, spv_diagnostic* diagnostic);
spv_result_t spvTextToBinaryWithOptions(const spv_const_context context,
const char* text, const size_t length,
const uint32_t options,
spv_binary* binary,
spv_diagnostic* diagnostic);
// Frees an allocated text stream. This is a no-op if the text parameter
// is a null pointer.

View File

@ -44,6 +44,10 @@ class ValidatorOptions {
spvValidatorOptionsSetUniversalLimit(options_, limit_type, limit);
}
void SetRelaxStructStore(bool val) {
spvValidatorOptionsSetRelaxStoreStruct(options_, val);
}
private:
spv_validator_options options_;
};

View File

@ -58,20 +58,26 @@ void spvValidatorOptionsSetUniversalLimit(spv_validator_options options,
spv_validator_limit limit_type,
uint32_t limit) {
assert(options && "Validator options object may not be Null");
switch(limit_type) {
switch (limit_type) {
#define LIMIT(TYPE, FIELD) \
case TYPE: \
options->universal_limits_.FIELD = limit; \
break;
LIMIT(spv_validator_limit_max_struct_members, max_struct_members)
LIMIT(spv_validator_limit_max_struct_depth, max_struct_depth)
LIMIT(spv_validator_limit_max_local_variables, max_local_variables)
LIMIT(spv_validator_limit_max_global_variables, max_global_variables)
LIMIT(spv_validator_limit_max_switch_branches, max_switch_branches)
LIMIT(spv_validator_limit_max_function_args, max_function_args)
LIMIT(spv_validator_limit_max_control_flow_nesting_depth,
max_control_flow_nesting_depth)
LIMIT(spv_validator_limit_max_access_chain_indexes, max_access_chain_indexes)
case TYPE: \
options->universal_limits_.FIELD = limit; \
break;
LIMIT(spv_validator_limit_max_struct_members, max_struct_members)
LIMIT(spv_validator_limit_max_struct_depth, max_struct_depth)
LIMIT(spv_validator_limit_max_local_variables, max_local_variables)
LIMIT(spv_validator_limit_max_global_variables, max_global_variables)
LIMIT(spv_validator_limit_max_switch_branches, max_switch_branches)
LIMIT(spv_validator_limit_max_function_args, max_function_args)
LIMIT(spv_validator_limit_max_control_flow_nesting_depth,
max_control_flow_nesting_depth)
LIMIT(spv_validator_limit_max_access_chain_indexes,
max_access_chain_indexes)
#undef LIMIT
}
}
void spvValidatorOptionsSetRelaxStoreStruct(spv_validator_options options,
bool val) {
options->relax_struct_store = val;
}

View File

@ -38,9 +38,10 @@ struct validator_universal_limits_t {
// members may be added for any new option.
struct spv_validator_options_t {
spv_validator_options_t()
: universal_limits_() {}
: universal_limits_(), relax_struct_store(false) {}
validator_universal_limits_t universal_limits_;
bool relax_struct_store;
};
#endif // LIBSPIRV_SPIRV_VALIDATOR_OPTIONS_H_

View File

@ -260,6 +260,9 @@ class ValidationState_t {
std::vector<Decoration>& id_decorations(uint32_t id) {
return id_decorations_[id];
}
const std::vector<Decoration>& id_decorations(uint32_t id) const {
return id_decorations_.at(id);
}
/// Finds id's def, if it exists. If found, returns the definition otherwise
/// nullptr

View File

@ -27,13 +27,13 @@
#include "message.h"
#include "opcode.h"
#include "operand.h"
#include "spirv_validator_options.h"
#include "spirv-tools/libspirv.h"
#include "spirv_validator_options.h"
#include "val/function.h"
#include "val/validation_state.h"
using libspirv::ValidationState_t;
using libspirv::Decoration;
using libspirv::ValidationState_t;
using std::function;
using std::ignore;
using std::make_pair;
@ -82,6 +82,26 @@ class idUsage {
const spvtools::MessageConsumer& consumer_;
const ValidationState_t& module_;
vector<uint32_t> entry_points_;
// Returns true if the two instructions represent structs that, as far as the
// validator can tell, have the exact same data layout.
bool AreLayoutCompatibleStructs(const libspirv::Instruction* type1,
const libspirv::Instruction* type2);
// Returns true if the operands to the OpTypeStruct instruction defining the
// types are the same or are layout compatible types. |type1| and |type2| must
// be OpTypeStruct instructions.
bool HaveLayoutCompatibleMembers(const libspirv::Instruction* type1,
const libspirv::Instruction* type2);
// Returns true if all decorations that affect the data layout of the struct
// (like Offset), are the same for the two types. |type1| and |type2| must be
// OpTypeStruct instructions.
bool HaveSameLayoutDecorations(const libspirv::Instruction* type1,
const libspirv::Instruction* type2);
bool HasConflictingMemberOffsets(
const vector<Decoration>& type1_decorations,
const vector<Decoration>& type2_decorations) const;
};
#define DIAG(INDEX) \
@ -207,17 +227,17 @@ bool idUsage::isValid<SpvOpGroupDecorate>(const spv_instruction_t* inst,
auto decorationGroupIndex = 1;
auto decorationGroup = module_.FindDef(inst->words[decorationGroupIndex]);
if (!decorationGroup || SpvOpDecorationGroup != decorationGroup->opcode()) {
DIAG(decorationGroupIndex) << "OpGroupDecorate Decoration group <id> '"
<< inst->words[decorationGroupIndex]
<< "' is not a decoration group.";
DIAG(decorationGroupIndex)
<< "OpGroupDecorate Decoration group <id> '"
<< inst->words[decorationGroupIndex] << "' is not a decoration group.";
return false;
}
return true;
}
template <>
bool idUsage::isValid<SpvOpGroupMemberDecorate>(
const spv_instruction_t* inst, const spv_opcode_desc) {
bool idUsage::isValid<SpvOpGroupMemberDecorate>(const spv_instruction_t* inst,
const spv_opcode_desc) {
auto decorationGroupIndex = 1;
auto decorationGroup = module_.FindDef(inst->words[decorationGroupIndex]);
if (!decorationGroup || SpvOpDecorationGroup != decorationGroup->opcode()) {
@ -275,9 +295,9 @@ bool idUsage::isValid<SpvOpEntryPoint>(const spv_instruction_t* inst,
// to change
auto entryPointType = module_.FindDef(entryPoint->words()[4]);
if (!entryPointType || 3 != entryPointType->words().size()) {
DIAG(entryPointIndex) << "OpEntryPoint Entry Point <id> '"
<< inst->words[entryPointIndex]
<< "'s function parameter count is not zero.";
DIAG(entryPointIndex)
<< "OpEntryPoint Entry Point <id> '" << inst->words[entryPointIndex]
<< "'s function parameter count is not zero.";
return false;
}
}
@ -436,9 +456,9 @@ bool idUsage::isValid<SpvOpTypeStruct>(const spv_instruction_t* inst,
auto memberTypeId = inst->words[memberTypeIndex];
auto memberType = module_.FindDef(memberTypeId);
if (!memberType || !spvOpcodeGeneratesType(memberType->opcode())) {
DIAG(memberTypeIndex) << "OpTypeStruct Member Type <id> '"
<< inst->words[memberTypeIndex]
<< "' is not a type.";
DIAG(memberTypeIndex)
<< "OpTypeStruct Member Type <id> '" << inst->words[memberTypeIndex]
<< "' is not a type.";
return false;
}
if (SpvOpTypeStruct == memberType->opcode() &&
@ -617,11 +637,11 @@ bool idUsage::isValid<SpvOpConstantComposite>(const spv_instruction_t* inst,
auto constituentResultType = module_.FindDef(constituent->type_id());
if (!constituentResultType ||
componentType->opcode() != constituentResultType->opcode()) {
DIAG(constituentIndex) << "OpConstantComposite Constituent <id> '"
<< inst->words[constituentIndex]
<< "'s type does not match Result Type <id> '"
<< resultType->id()
<< "'s vector element type.";
DIAG(constituentIndex)
<< "OpConstantComposite Constituent <id> '"
<< inst->words[constituentIndex]
<< "'s type does not match Result Type <id> '" << resultType->id()
<< "'s vector element type.";
return false;
}
}
@ -646,9 +666,8 @@ bool idUsage::isValid<SpvOpConstantComposite>(const spv_instruction_t* inst,
for (size_t constituentIndex = 3; constituentIndex < inst->words.size();
constituentIndex++) {
auto constituent = module_.FindDef(inst->words[constituentIndex]);
if (!constituent ||
!(SpvOpConstantComposite == constituent->opcode() ||
SpvOpUndef == constituent->opcode())) {
if (!constituent || !(SpvOpConstantComposite == constituent->opcode() ||
SpvOpUndef == constituent->opcode())) {
// The message says "... or undef" because the spec does not say
// undef is a constant.
DIAG(constituentIndex) << "OpConstantComposite Constituent <id> '"
@ -659,11 +678,11 @@ bool idUsage::isValid<SpvOpConstantComposite>(const spv_instruction_t* inst,
auto vector = module_.FindDef(constituent->type_id());
assert(vector);
if (columnType->opcode() != vector->opcode()) {
DIAG(constituentIndex) << "OpConstantComposite Constituent <id> '"
<< inst->words[constituentIndex]
<< "' type does not match Result Type <id> '"
<< resultType->id()
<< "'s matrix column type.";
DIAG(constituentIndex)
<< "OpConstantComposite Constituent <id> '"
<< inst->words[constituentIndex]
<< "' type does not match Result Type <id> '" << resultType->id()
<< "'s matrix column type.";
return false;
}
auto vectorComponentType = module_.FindDef(vector->words()[2]);
@ -711,11 +730,11 @@ bool idUsage::isValid<SpvOpConstantComposite>(const spv_instruction_t* inst,
auto constituentType = module_.FindDef(constituent->type_id());
assert(constituentType);
if (elementType->id() != constituentType->id()) {
DIAG(constituentIndex) << "OpConstantComposite Constituent <id> '"
<< inst->words[constituentIndex]
<< "'s type does not match Result Type <id> '"
<< resultType->id()
<< "'s array element type.";
DIAG(constituentIndex)
<< "OpConstantComposite Constituent <id> '"
<< inst->words[constituentIndex]
<< "'s type does not match Result Type <id> '" << resultType->id()
<< "'s array element type.";
return false;
}
}
@ -873,12 +892,13 @@ bool idUsage::isValid<SpvOpSampledImage>(const spv_instruction_t* inst,
auto consumer_opcode = consumer_instr->opcode();
if (consumer_instr->block() != sampledImageInstr->block()) {
DIAG(resultTypeIndex)
<< "All OpSampledImage instructions must be in the same block in "
<< "All OpSampledImage instructions must be in the same block in "
"which their Result <id> are consumed. OpSampledImage Result "
"Type <id> '"
<< resultID << "' has a consumer in a different basic "
"block. The consumer instruction <id> is '"
<< consumer_id << "'.";
<< resultID
<< "' has a consumer in a different basic "
"block. The consumer instruction <id> is '"
<< consumer_id << "'.";
return false;
}
// TODO: The following check is incomplete. We should also check that the
@ -945,11 +965,11 @@ bool idUsage::isValid<SpvOpSpecConstantComposite>(const spv_instruction_t* inst,
auto constituentResultType = module_.FindDef(constituent->type_id());
if (!constituentResultType ||
componentType->opcode() != constituentResultType->opcode()) {
DIAG(constituentIndex) << "OpSpecConstantComposite Constituent <id> '"
<< inst->words[constituentIndex]
<< "'s type does not match Result Type <id> '"
<< resultType->id()
<< "'s vector element type.";
DIAG(constituentIndex)
<< "OpSpecConstantComposite Constituent <id> '"
<< inst->words[constituentIndex]
<< "'s type does not match Result Type <id> '" << resultType->id()
<< "'s vector element type.";
return false;
}
}
@ -975,10 +995,9 @@ bool idUsage::isValid<SpvOpSpecConstantComposite>(const spv_instruction_t* inst,
constituentIndex++) {
auto constituent = module_.FindDef(inst->words[constituentIndex]);
auto constituentOpCode = constituent->opcode();
if (!constituent ||
!(SpvOpSpecConstantComposite == constituentOpCode ||
SpvOpConstantComposite == constituentOpCode ||
SpvOpUndef == constituentOpCode)) {
if (!constituent || !(SpvOpSpecConstantComposite == constituentOpCode ||
SpvOpConstantComposite == constituentOpCode ||
SpvOpUndef == constituentOpCode)) {
// The message says "... or undef" because the spec does not say
// undef is a constant.
DIAG(constituentIndex) << "OpSpecConstantComposite Constituent <id> '"
@ -989,11 +1008,11 @@ bool idUsage::isValid<SpvOpSpecConstantComposite>(const spv_instruction_t* inst,
auto vector = module_.FindDef(constituent->type_id());
assert(vector);
if (columnType->opcode() != vector->opcode()) {
DIAG(constituentIndex) << "OpSpecConstantComposite Constituent <id> '"
<< inst->words[constituentIndex]
<< "' type does not match Result Type <id> '"
<< resultType->id()
<< "'s matrix column type.";
DIAG(constituentIndex)
<< "OpSpecConstantComposite Constituent <id> '"
<< inst->words[constituentIndex]
<< "' type does not match Result Type <id> '" << resultType->id()
<< "'s matrix column type.";
return false;
}
auto vectorComponentType = module_.FindDef(vector->words()[2]);
@ -1042,11 +1061,11 @@ bool idUsage::isValid<SpvOpSpecConstantComposite>(const spv_instruction_t* inst,
auto constituentType = module_.FindDef(constituent->type_id());
assert(constituentType);
if (elementType->id() != constituentType->id()) {
DIAG(constituentIndex) << "OpSpecConstantComposite Constituent <id> '"
<< inst->words[constituentIndex]
<< "'s type does not match Result Type <id> '"
<< resultType->id()
<< "'s array element type.";
DIAG(constituentIndex)
<< "OpSpecConstantComposite Constituent <id> '"
<< inst->words[constituentIndex]
<< "'s type does not match Result Type <id> '" << resultType->id()
<< "'s array element type.";
return false;
}
}
@ -1119,9 +1138,9 @@ bool idUsage::isValid<SpvOpVariable>(const spv_instruction_t* inst,
const auto is_constant =
initialiser && spvOpcodeIsConstant(initialiser->opcode());
if (!initialiser || !(is_constant || is_module_scope_var)) {
DIAG(initialiserIndex) << "OpVariable Initializer <id> '"
<< inst->words[initialiserIndex]
<< "' is not a constant or module-scope variable.";
DIAG(initialiserIndex)
<< "OpVariable Initializer <id> '" << inst->words[initialiserIndex]
<< "' is not a constant or module-scope variable.";
return false;
}
}
@ -1220,10 +1239,24 @@ bool idUsage::isValid<SpvOpStore>(const spv_instruction_t* inst,
}
if (type->id() != objectType->id()) {
DIAG(pointerIndex) << "OpStore Pointer <id> '" << inst->words[pointerIndex]
<< "'s type does not match Object <id> '"
<< objectType->id() << "'s type.";
return false;
if (!module_.options()->relax_struct_store
|| type->opcode() != SpvOpTypeStruct
|| objectType->opcode() != SpvOpTypeStruct) {
DIAG(pointerIndex) << "OpStore Pointer <id> '"
<< inst->words[pointerIndex]
<< "'s type does not match Object <id> '"
<< object->id() << "'s type.";
return false;
}
// TODO: Check for layout compatible matricies and arrays as well.
if (!AreLayoutCompatibleStructs(type, objectType)) {
DIAG(pointerIndex) << "OpStore Pointer <id> '"
<< inst->words[pointerIndex]
<< "'s layout does not match Object <id> '"
<< object->id() << "'s layout.";
return false;
}
}
return true;
}
@ -1440,8 +1473,9 @@ bool idUsage::isValid<SpvOpAccessChain>(const spv_instruction_t* inst,
}
default: {
// Give an error. reached non-composite type while indexes still remain.
DIAG(i) << instr_name << " reached non-composite type while indexes "
"still remain to be traversed.";
DIAG(i) << instr_name
<< " reached non-composite type while indexes "
"still remain to be traversed.";
return false;
}
}
@ -1516,9 +1550,9 @@ bool idUsage::isValid<SpvOpFunction>(const spv_instruction_t* inst,
auto functionTypeIndex = 4;
auto functionType = module_.FindDef(inst->words[functionTypeIndex]);
if (!functionType || SpvOpTypeFunction != functionType->opcode()) {
DIAG(functionTypeIndex) << "OpFunction Function Type <id> '"
<< inst->words[functionTypeIndex]
<< "' is not a function type.";
DIAG(functionTypeIndex)
<< "OpFunction Function Type <id> '" << inst->words[functionTypeIndex]
<< "' is not a function type.";
return false;
}
auto returnType = module_.FindDef(functionType->words()[2]);
@ -1774,8 +1808,9 @@ bool walkCompositeTypeHierarchy(
}
default: {
// Give an error. reached non-composite type while indexes still remain.
*error << instr_name() << " reached non-composite type while indexes "
"still remain to be traversed.";
*error << instr_name()
<< " reached non-composite type while indexes "
"still remain to be traversed.";
return false;
}
}
@ -1900,12 +1935,13 @@ bool idUsage::isValid<SpvOpCompositeInsert>(const spv_instruction_t* inst,
auto objectTypeInstr = module_.FindDef(objectInstr->type_id());
if (indexedTypeInstr->id() != objectTypeInstr->id()) {
DIAG(objectIdIndex)
<< "The Object type (Op"
<< spvOpcodeString(static_cast<SpvOp>(objectTypeInstr->opcode()))
<< ") in " << instr_name() << " does not match the type that results "
"from indexing into the Composite (Op"
<< spvOpcodeString(static_cast<SpvOp>(indexedTypeInstr->opcode()))
<< ").";
<< "The Object type (Op"
<< spvOpcodeString(static_cast<SpvOp>(objectTypeInstr->opcode()))
<< ") in " << instr_name()
<< " does not match the type that results "
"from indexing into the Composite (Op"
<< spvOpcodeString(static_cast<SpvOp>(indexedTypeInstr->opcode()))
<< ").";
return false;
}
@ -2592,6 +2628,102 @@ bool idUsage::isValid(const spv_instruction_t* inst) {
#undef TODO
#undef CASE
}
bool idUsage::AreLayoutCompatibleStructs(const libspirv::Instruction* type1,
const libspirv::Instruction* type2) {
if (type1->opcode() != SpvOpTypeStruct) {
return false;
}
if (type2->opcode() != SpvOpTypeStruct) {
return false;
}
if (!HaveLayoutCompatibleMembers(type1, type2)) return false;
return HaveSameLayoutDecorations(type1, type2);
}
bool idUsage::HaveLayoutCompatibleMembers(const libspirv::Instruction* type1,
const libspirv::Instruction* type2) {
assert(type1->opcode() == SpvOpTypeStruct &&
"type1 must be and OpTypeStruct instruction.");
assert(type2->opcode() == SpvOpTypeStruct &&
"type2 must be and OpTypeStruct instruction.");
const auto& type1_operands = type1->operands();
const auto& type2_operands = type2->operands();
if (type1_operands.size() != type2_operands.size()) {
return false;
}
for (size_t operand = 2; operand < type1_operands.size(); ++operand) {
if (type1->word(operand) != type2->word(operand)) {
auto def1 = module_.FindDef(type1->word(operand));
auto def2 = module_.FindDef(type2->word(operand));
if (!AreLayoutCompatibleStructs(def1, def2)) {
return false;
}
}
}
return true;
}
bool idUsage::HaveSameLayoutDecorations(const libspirv::Instruction* type1,
const libspirv::Instruction* type2) {
assert(type1->opcode() == SpvOpTypeStruct &&
"type1 must be and OpTypeStruct instruction.");
assert(type2->opcode() == SpvOpTypeStruct &&
"type2 must be and OpTypeStruct instruction.");
const std::vector<Decoration>& type1_decorations =
module_.id_decorations(type1->id());
const std::vector<Decoration>& type2_decorations =
module_.id_decorations(type2->id());
// TODO: Will have to add other check for arrays an matricies if we want to
// handle them.
if (HasConflictingMemberOffsets(type1_decorations, type2_decorations)) {
return false;
}
return true;
}
bool idUsage::HasConflictingMemberOffsets(
const vector<Decoration>& type1_decorations,
const vector<Decoration>& type2_decorations) const {
{
// We are interested in conflicting decoration. If a decoration is in one
// list but not the other, then we will assume the code is correct. We are
// looking for things we know to be wrong.
//
// We do not have to traverse type2_decoration because, after traversing
// type1_decorations, anything new will not be found in
// type1_decoration. Therefore, it cannot lead to a conflict.
for (const Decoration& decoration : type1_decorations) {
switch (decoration.dec_type()) {
case SpvDecorationOffset: {
// Since these affect the layout of the struct, they must be present
// in both structs.
auto compare = [&decoration](const Decoration& rhs) {
if (rhs.dec_type() != SpvDecorationOffset) return false;
return decoration.struct_member_index() ==
rhs.struct_member_index();
};
auto i = find_if(type2_decorations.begin(), type2_decorations.end(),
compare);
if (i != type2_decorations.end() &&
decoration.params().front() != i->params().front()) {
return true;
}
} break;
default:
// This decoration does not affect the layout of the structure, so
// just moving on.
break;
}
}
}
return false;
}
} // anonymous namespace
namespace libspirv {
@ -2729,9 +2861,9 @@ spv_result_t IdPass(ValidationState_t& _,
} else if (can_have_forward_declared_ids(i)) {
ret = _.ForwardDeclareId(operand_word);
} else {
ret = _.diag(SPV_ERROR_INVALID_ID) << "ID "
<< _.getIdName(operand_word)
<< " has not been defined";
ret = _.diag(SPV_ERROR_INVALID_ID)
<< "ID " << _.getIdName(operand_word)
<< " has not been defined";
}
break;
default:

View File

@ -28,8 +28,8 @@
namespace {
using ::testing::ValuesIn;
using ::testing::HasSubstr;
using ::testing::ValuesIn;
using spvtest::ScopedContext;
using std::ostringstream;
using std::string;
@ -381,7 +381,6 @@ TEST_F(ValidateIdWithMessage, OpEntryPointInterfaceIsNotVariableTypeBad) {
"OpTypeVariable. Found OpTypePointer."));
}
TEST_F(ValidateIdWithMessage, OpEntryPointInterfaceStorageClassBad) {
string spirv = R"(
OpCapability Shader
@ -578,7 +577,7 @@ class OpTypeArrayLengthTest
~OpTypeArrayLengthTest() { spvDiagnosticDestroy(diagnostic_); }
// Runs spvValidate() on v, printing any errors via spvDiagnosticPrint().
spv_result_t Val(const SpirvVector& v, const std::string &expected_err = "") {
spv_result_t Val(const SpirvVector& v, const std::string& expected_err = "") {
spv_const_binary_t cbinary{v.data(), v.size()};
const auto status =
spvValidate(ScopedContext().context, &cbinary, &diagnostic_);
@ -1971,8 +1970,7 @@ TEST_F(ValidateIdWithMessage, OpLoadVarPtrOpFunctionCallGood) {
%result = OpLoad %f32 %varptr
)";
createVariablePointerSpirvProgram(&spirv,
result_strategy,
createVariablePointerSpirvProgram(&spirv, result_strategy,
true /* Add VariablePointers Capability?*/,
true /* Use Helper Function? */);
CompileSuccessfully(spirv.str());
@ -2015,8 +2013,7 @@ TEST_F(ValidateIdWithMessage, OpLoadPointerBad) {
EXPECT_EQ(SPV_ERROR_INVALID_ID, ValidateInstructions());
// Prove that SSA checks trigger for a bad Id value.
// The next test case show the not-a-logical-pointer case.
EXPECT_THAT(getDiagnosticString(),
HasSubstr("ID 8 has not been defined"));
EXPECT_THAT(getDiagnosticString(), HasSubstr("ID 8 has not been defined"));
}
// Disabled as bitcasting type to object is now not valid.
@ -2176,7 +2173,211 @@ TEST_F(ValidateIdWithMessage, OpStoreTypeBad) {
EXPECT_EQ(SPV_ERROR_INVALID_ID, ValidateInstructions());
EXPECT_THAT(getDiagnosticString(),
HasSubstr("OpStore Pointer <id> '7's type does not match Object "
"<id> '3's type."));
"<id> '6's type."));
}
// The next series of test check test a relaxation of the rules for stores to
// structs. The first test checks that we get a failure when the option is not
// set to relax the rule.
// TODO: Add tests for layout compatible arrays and matricies when the validator
// relaxes the rules for them as well. Also need test to check for layout
// decorations specific to those types.
TEST_F(ValidateIdWithMessage, OpStoreTypeBadStruct) {
string spirv = kGLSL450MemoryModel + R"(
OpMemberDecorate %1 0 Offset 0
OpMemberDecorate %1 1 Offset 4
OpMemberDecorate %2 0 Offset 0
OpMemberDecorate %2 1 Offset 4
%3 = OpTypeVoid
%4 = OpTypeFloat 32
%1 = OpTypeStruct %4 %4
%5 = OpTypePointer Uniform %1
%2 = OpTypeStruct %4 %4
%6 = OpTypeFunction %3
%7 = OpConstant %4 3.14
%8 = OpVariable %5 Uniform
%9 = OpFunction %3 None %6
%10 = OpLabel
%11 = OpCompositeConstruct %2 %7 %7
OpStore %8 %11
OpReturn
OpFunctionEnd)";
CompileSuccessfully(spirv.c_str());
EXPECT_EQ(SPV_ERROR_INVALID_ID, ValidateInstructions());
EXPECT_THAT(getDiagnosticString(),
HasSubstr("OpStore Pointer <id> '8's type does not match Object "
"<id> '11's type."));
}
// Same code as the last test. The difference is that we relax the rule.
// Because the structs %3 and %5 are defined the same way.
TEST_F(ValidateIdWithMessage, OpStoreTypeRelaxedStruct) {
string spirv = kGLSL450MemoryModel + R"(
OpMemberDecorate %1 0 Offset 0
OpMemberDecorate %1 1 Offset 4
OpMemberDecorate %2 0 Offset 0
OpMemberDecorate %2 1 Offset 4
%3 = OpTypeVoid
%4 = OpTypeFloat 32
%1 = OpTypeStruct %4 %4
%5 = OpTypePointer Uniform %1
%2 = OpTypeStruct %4 %4
%6 = OpTypeFunction %3
%7 = OpConstant %4 3.14
%8 = OpVariable %5 Uniform
%9 = OpFunction %3 None %6
%10 = OpLabel
%11 = OpCompositeConstruct %2 %7 %7
OpStore %8 %11
OpReturn
OpFunctionEnd)";
spvValidatorOptionsSetRelaxStoreStruct(options_, true);
CompileSuccessfully(spirv.c_str());
EXPECT_EQ(SPV_SUCCESS, ValidateInstructions());
}
// Same code as the last test excect for an extra decoration on one of the
// members. With the relaxed rules, the code is still valid.
TEST_F(ValidateIdWithMessage, OpStoreTypeRelaxedStructWithExtraDecoration) {
string spirv = kGLSL450MemoryModel + R"(
OpMemberDecorate %1 0 Offset 0
OpMemberDecorate %1 1 Offset 4
OpMemberDecorate %1 0 RelaxedPrecision
OpMemberDecorate %2 0 Offset 0
OpMemberDecorate %2 1 Offset 4
%3 = OpTypeVoid
%4 = OpTypeFloat 32
%1 = OpTypeStruct %4 %4
%5 = OpTypePointer Uniform %1
%2 = OpTypeStruct %4 %4
%6 = OpTypeFunction %3
%7 = OpConstant %4 3.14
%8 = OpVariable %5 Uniform
%9 = OpFunction %3 None %6
%10 = OpLabel
%11 = OpCompositeConstruct %2 %7 %7
OpStore %8 %11
OpReturn
OpFunctionEnd)";
spvValidatorOptionsSetRelaxStoreStruct(options_, true);
CompileSuccessfully(spirv.c_str());
EXPECT_EQ(SPV_SUCCESS, ValidateInstructions());
}
// This test check that we recursively traverse the struct to check if they are
// interchangable.
TEST_F(ValidateIdWithMessage, OpStoreTypeRelaxedNestedStruct) {
string spirv = kGLSL450MemoryModel + R"(
OpMemberDecorate %1 0 Offset 0
OpMemberDecorate %1 1 Offset 4
OpMemberDecorate %2 0 Offset 0
OpMemberDecorate %2 1 Offset 8
OpMemberDecorate %3 0 Offset 0
OpMemberDecorate %3 1 Offset 4
OpMemberDecorate %4 0 Offset 0
OpMemberDecorate %4 1 Offset 8
%5 = OpTypeVoid
%6 = OpTypeInt 32 0
%7 = OpTypeFloat 32
%1 = OpTypeStruct %7 %6
%2 = OpTypeStruct %1 %1
%8 = OpTypePointer Uniform %2
%3 = OpTypeStruct %7 %6
%4 = OpTypeStruct %3 %3
%9 = OpTypeFunction %5
%10 = OpConstant %6 7
%11 = OpConstant %7 3.14
%12 = OpVariable %8 Uniform
%13 = OpFunction %5 None %9
%14 = OpLabel
%15 = OpCompositeConstruct %3 %11 %6
%16 = OpCompositeConstruct %4 %11 %11
OpStore %12 %16
OpReturn
OpFunctionEnd)";
spvValidatorOptionsSetRelaxStoreStruct(options_, true);
CompileSuccessfully(spirv.c_str());
EXPECT_EQ(SPV_SUCCESS, ValidateInstructions());
}
// This test check that the even with the relaxed rules an error is identified
// if the members of the struct are in a different order.
TEST_F(ValidateIdWithMessage, OpStoreTypeBadRelaxedStruct1) {
string spirv = kGLSL450MemoryModel + R"(
OpMemberDecorate %1 0 Offset 0
OpMemberDecorate %1 1 Offset 4
OpMemberDecorate %2 0 Offset 0
OpMemberDecorate %2 1 Offset 8
OpMemberDecorate %3 0 Offset 0
OpMemberDecorate %3 1 Offset 4
OpMemberDecorate %4 0 Offset 0
OpMemberDecorate %4 1 Offset 8
%5 = OpTypeVoid
%6 = OpTypeInt 32 0
%7 = OpTypeFloat 32
%1 = OpTypeStruct %6 %7
%2 = OpTypeStruct %1 %1
%8 = OpTypePointer Uniform %2
%3 = OpTypeStruct %7 %6
%4 = OpTypeStruct %3 %3
%9 = OpTypeFunction %5
%10 = OpConstant %6 7
%11 = OpConstant %7 3.14
%12 = OpVariable %8 Uniform
%13 = OpFunction %5 None %9
%14 = OpLabel
%15 = OpCompositeConstruct %3 %11 %6
%16 = OpCompositeConstruct %4 %11 %11
OpStore %12 %16
OpReturn
OpFunctionEnd)";
spvValidatorOptionsSetRelaxStoreStruct(options_, true);
CompileSuccessfully(spirv.c_str());
EXPECT_EQ(SPV_ERROR_INVALID_ID, ValidateInstructions());
EXPECT_THAT(
getDiagnosticString(),
HasSubstr("OpStore Pointer <id> '12's layout does not match Object "
"<id> '16's layout."));
}
// This test check that the even with the relaxed rules an error is identified
// if the members of the struct are at different offsets.
TEST_F(ValidateIdWithMessage, OpStoreTypeBadRelaxedStruct2) {
string spirv = kGLSL450MemoryModel + R"(
OpMemberDecorate %1 0 Offset 4
OpMemberDecorate %1 1 Offset 0
OpMemberDecorate %2 0 Offset 0
OpMemberDecorate %2 1 Offset 8
OpMemberDecorate %3 0 Offset 0
OpMemberDecorate %3 1 Offset 4
OpMemberDecorate %4 0 Offset 0
OpMemberDecorate %4 1 Offset 8
%5 = OpTypeVoid
%6 = OpTypeInt 32 0
%7 = OpTypeFloat 32
%1 = OpTypeStruct %7 %6
%2 = OpTypeStruct %1 %1
%8 = OpTypePointer Uniform %2
%3 = OpTypeStruct %7 %6
%4 = OpTypeStruct %3 %3
%9 = OpTypeFunction %5
%10 = OpConstant %6 7
%11 = OpConstant %7 3.14
%12 = OpVariable %8 Uniform
%13 = OpFunction %5 None %9
%14 = OpLabel
%15 = OpCompositeConstruct %3 %11 %6
%16 = OpCompositeConstruct %4 %11 %11
OpStore %12 %16
OpReturn
OpFunctionEnd)";
spvValidatorOptionsSetRelaxStoreStruct(options_, true);
CompileSuccessfully(spirv.c_str());
EXPECT_EQ(SPV_ERROR_INVALID_ID, ValidateInstructions());
EXPECT_THAT(
getDiagnosticString(),
HasSubstr("OpStore Pointer <id> '12's layout does not match Object "
"<id> '16's layout."));
}
TEST_F(ValidateIdWithMessage, OpStoreVoid) {
@ -2450,7 +2651,8 @@ TEST_P(AccessChainInstructionTest, AccessChainGood) {
const std::string elem = AccessChainRequiresElemId(instr) ? "%int_0 " : "";
string spirv = kGLSL450MemoryModel + kDeeplyNestedStructureSetup +
"%float_entry = " + instr +
R"( %_ptr_Private_float %my_matrix )" + elem + R"(%int_0 %int_1
R"( %_ptr_Private_float %my_matrix )" + elem +
R"(%int_0 %int_1
OpReturn
OpFunctionEnd
)";
@ -2483,7 +2685,8 @@ TEST_P(AccessChainInstructionTest, AccessChainBaseTypeVoidBad) {
const std::string elem = AccessChainRequiresElemId(instr) ? "%int_0 " : "";
string spirv = kGLSL450MemoryModel + kDeeplyNestedStructureSetup + R"(
%float_entry = )" +
instr + " %_ptr_Private_float %void " + elem + R"(%int_0 %int_1
instr + " %_ptr_Private_float %void " + elem +
R"(%int_0 %int_1
OpReturn
OpFunctionEnd
)";
@ -2500,7 +2703,8 @@ TEST_P(AccessChainInstructionTest, AccessChainBaseTypeNonPtrVariableBad) {
const std::string instr = GetParam();
const std::string elem = AccessChainRequiresElemId(instr) ? "%int_0 " : "";
string spirv = kGLSL450MemoryModel + kDeeplyNestedStructureSetup + R"(
%entry = )" + instr +
%entry = )" +
instr +
R"( %_ptr_Private_float %_ptr_Private_float )" + elem +
R"(%int_0 %int_1
OpReturn
@ -2520,7 +2724,8 @@ TEST_P(AccessChainInstructionTest,
const std::string instr = GetParam();
const std::string elem = AccessChainRequiresElemId(instr) ? "%int_0 " : "";
string spirv = kGLSL450MemoryModel + kDeeplyNestedStructureSetup + R"(
%entry = )" + instr +
%entry = )" +
instr +
R"( %_ptr_Function_float %my_matrix )" + elem +
R"(%int_0 %int_1
OpReturn
@ -2541,7 +2746,8 @@ TEST_P(AccessChainInstructionTest,
const std::string instr = GetParam();
const std::string elem = AccessChainRequiresElemId(instr) ? "%int_0 " : "";
string spirv = kGLSL450MemoryModel + kDeeplyNestedStructureSetup + R"(
%entry = )" + instr +
%entry = )" +
instr +
R"( %_ptr_Private_float %my_float_var )" + elem + R"(%int_0
OpReturn
OpFunctionEnd
@ -2560,7 +2766,8 @@ TEST_P(AccessChainInstructionTest, AccessChainNoIndexesGood) {
const std::string instr = GetParam();
const std::string elem = AccessChainRequiresElemId(instr) ? "%int_0 " : "";
string spirv = kGLSL450MemoryModel + kDeeplyNestedStructureSetup + R"(
%entry = )" + instr +
%entry = )" +
instr +
R"( %_ptr_Private_float %my_float_var )" + elem + R"(
OpReturn
OpFunctionEnd
@ -2575,7 +2782,8 @@ TEST_P(AccessChainInstructionTest, AccessChainNoIndexesBad) {
const std::string instr = GetParam();
const std::string elem = AccessChainRequiresElemId(instr) ? "%int_0 " : "";
string spirv = kGLSL450MemoryModel + kDeeplyNestedStructureSetup + R"(
%entry = )" + instr +
%entry = )" +
instr +
R"( %_ptr_Private_mat4x3 %my_float_var )" + elem + R"(
OpReturn
OpFunctionEnd
@ -2724,9 +2932,10 @@ TEST_P(AccessChainInstructionTest, CustomizedAccessChainTooManyIndexesBad) {
TEST_P(AccessChainInstructionTest, AccessChainUndefinedIndexBad) {
const std::string instr = GetParam();
const std::string elem = AccessChainRequiresElemId(instr) ? "%int_0 " : "";
string spirv = kGLSL450MemoryModel + kDeeplyNestedStructureSetup + R"(
string spirv =
kGLSL450MemoryModel + kDeeplyNestedStructureSetup + R"(
%entry = )" + instr +
R"( %_ptr_Private_float %my_matrix )" + elem + R"(%float %int_1
R"( %_ptr_Private_float %my_matrix )" + elem + R"(%float %int_1
OpReturn
OpFunctionEnd
)";
@ -2743,8 +2952,9 @@ TEST_P(AccessChainInstructionTest, AccessChainStructIndexNotConstantBad) {
const std::string instr = GetParam();
const std::string elem = AccessChainRequiresElemId(instr) ? "%int_0 " : "";
string spirv = kGLSL450MemoryModel + kDeeplyNestedStructureSetup + R"(
%f = )" + instr + R"( %_ptr_Uniform_float %blockName_var )" +
elem + R"(%int_0 %spec_int %int_2
%f = )" +
instr + R"( %_ptr_Uniform_float %blockName_var )" + elem +
R"(%int_0 %spec_int %int_2
OpReturn
OpFunctionEnd
)";
@ -2762,7 +2972,8 @@ TEST_P(AccessChainInstructionTest,
const std::string instr = GetParam();
const std::string elem = AccessChainRequiresElemId(instr) ? "%int_0 " : "";
string spirv = kGLSL450MemoryModel + kDeeplyNestedStructureSetup + R"(
%entry = )" + instr +
%entry = )" +
instr +
R"( %_ptr_Uniform_float %blockName_var )" + elem +
R"(%int_0 %int_1 %int_2
OpReturn
@ -2782,7 +2993,8 @@ TEST_P(AccessChainInstructionTest, AccessChainStructTooManyIndexesBad) {
const std::string instr = GetParam();
const std::string elem = AccessChainRequiresElemId(instr) ? "%int_0 " : "";
string spirv = kGLSL450MemoryModel + kDeeplyNestedStructureSetup + R"(
%entry = )" + instr +
%entry = )" +
instr +
R"( %_ptr_Uniform_float %blockName_var )" + elem +
R"(%int_0 %int_2 %int_2
OpReturn
@ -2801,7 +3013,8 @@ TEST_P(AccessChainInstructionTest, AccessChainStructIndexOutOfBoundBad) {
const std::string instr = GetParam();
const std::string elem = AccessChainRequiresElemId(instr) ? "%int_0 " : "";
string spirv = kGLSL450MemoryModel + kDeeplyNestedStructureSetup + R"(
%entry = )" + instr +
%entry = )" +
instr +
R"( %_ptr_Uniform_float %blockName_var )" + elem +
R"(%int_3 %int_2 %int_2
OpReturn
@ -2888,7 +3101,8 @@ TEST_P(AccessChainInstructionTest, AccessChainMatrixMoreArgsThanNeededBad) {
const std::string instr = GetParam();
const std::string elem = AccessChainRequiresElemId(instr) ? "%int_0 " : "";
string spirv = kGLSL450MemoryModel + kDeeplyNestedStructureSetup + R"(
%entry = )" + instr +
%entry = )" +
instr +
R"( %_ptr_Private_float %my_matrix )" + elem +
R"(%int_0 %int_1 %int_0
OpReturn
@ -2908,7 +3122,8 @@ TEST_P(AccessChainInstructionTest,
const std::string instr = GetParam();
const std::string elem = AccessChainRequiresElemId(instr) ? "%int_0 " : "";
string spirv = kGLSL450MemoryModel + kDeeplyNestedStructureSetup + R"(
%entry = )" + instr +
%entry = )" +
instr +
R"( %_ptr_Private_mat4x3 %my_matrix )" + elem +
R"(%int_0 %int_1
OpReturn
@ -4086,7 +4301,7 @@ TEST_F(ValidateIdWithMessage, OpReturnValueIsVariableInLogical) {
TEST_F(ValidateIdWithMessage, OpReturnValueVarPtrGood) {
std::ostringstream spirv;
createVariablePointerSpirvProgram(&spirv,
"" /* Instructions to add to "main" */,
"" /* Instructions to add to "main" */,
true /* Add VariablePointers Capability?*/,
true /* Use Helper Function? */);
CompileSuccessfully(spirv.str());
@ -4100,9 +4315,9 @@ TEST_F(ValidateIdWithMessage, OpReturnValueVarPtrGood) {
TEST_F(ValidateIdWithMessage, DISABLED_OpReturnValueVarPtrBad) {
std::ostringstream spirv;
createVariablePointerSpirvProgram(&spirv,
"" /* Instructions to add to "main" */,
"" /* Instructions to add to "main" */,
false /* Add VariablePointers Capability?*/,
true /* Use Helper Function? */);
true /* Use Helper Function? */);
CompileSuccessfully(spirv.str());
EXPECT_EQ(SPV_ERROR_INVALID_ID, ValidateInstructions());
EXPECT_THAT(getDiagnosticString(),

View File

@ -12,6 +12,7 @@
// See the License for the specific language governing permissions and
// limitations under the License.
#include <spirv_validator_options.h>
#include <algorithm>
#include <cassert>
#include <cstring>
@ -155,6 +156,10 @@ Options:
Replaces instructions with equivalent and less expensive ones.
--eliminate-dead-variables
Deletes module scope variables that are not referenced.
--relax-store-struct
Allow store from one struct type to a different type with
compatible layout and members. This option is forwarded to the
validator.
-O
Optimize for performance. Apply a sequence of transformations
in an attempt to improve the performance of the generated
@ -242,7 +247,8 @@ bool ReadFlagsFromFile(const char* oconfig_flag,
}
OptStatus ParseFlags(int argc, const char** argv, Optimizer* optimizer,
const char** in_file, const char** out_file);
const char** in_file, const char** out_file,
spv_validator_options options);
// Parses and handles the -Oconfig flag. |prog_name| contains the name of
// the spirv-opt binary (used to build a new argv vector for the recursive
@ -276,7 +282,7 @@ OptStatus ParseOconfigFlag(const char* prog_name, const char* opt_flag,
}
return ParseFlags(static_cast<int>(flags.size()), new_argv, optimizer,
in_file, out_file);
in_file, out_file, nullptr);
}
// Parses command-line flags. |argc| contains the number of command-line flags.
@ -288,7 +294,8 @@ OptStatus ParseOconfigFlag(const char* prog_name, const char* opt_flag,
// optimization should continue and a status code indicating an error or
// success.
OptStatus ParseFlags(int argc, const char** argv, Optimizer* optimizer,
const char** in_file, const char** out_file) {
const char** in_file, const char** out_file,
spv_validator_options options) {
for (int argi = 1; argi < argc; ++argi) {
const char* cur_arg = argv[argi];
if ('-' == cur_arg[0]) {
@ -369,6 +376,8 @@ OptStatus ParseFlags(int argc, const char** argv, Optimizer* optimizer,
optimizer->RegisterPass(CreateCompactIdsPass());
} else if (0 == strcmp(cur_arg, "--cfg-cleanup")) {
optimizer->RegisterPass(CreateCFGCleanupPass());
} else if (0 == strcmp(cur_arg, "--relax-store-struct")) {
options->relax_struct_store = true;
} else if (0 == strcmp(cur_arg, "-O")) {
optimizer->RegisterPerformancePasses();
} else if (0 == strcmp(cur_arg, "-Os")) {
@ -414,6 +423,7 @@ int main(int argc, const char** argv) {
const char* out_file = nullptr;
spv_target_env target_env = SPV_ENV_UNIVERSAL_1_2;
spv_validator_options options = spvValidatorOptionsCreate();
spvtools::Optimizer optimizer(target_env);
optimizer.SetMessageConsumer([](spv_message_level_t level, const char* source,
@ -423,7 +433,9 @@ int main(int argc, const char** argv) {
<< std::endl;
});
OptStatus status = ParseFlags(argc, argv, &optimizer, &in_file, &out_file);
OptStatus status =
ParseFlags(argc, argv, &optimizer, &in_file, &out_file, options);
if (status.action == OPT_STOP) {
return status.code;
}
@ -442,14 +454,17 @@ int main(int argc, const char** argv) {
spv_context context = spvContextCreate(target_env);
spv_diagnostic diagnostic = nullptr;
spv_const_binary_t binary_struct = {binary.data(), binary.size()};
spv_result_t error = spvValidate(context, &binary_struct, &diagnostic);
spv_result_t error =
spvValidateWithOptions(context, options, &binary_struct, &diagnostic);
if (error) {
spvDiagnosticPrint(diagnostic);
spvDiagnosticDestroy(diagnostic);
spvValidatorOptionsDestroy(options);
spvContextDestroy(context);
return error;
}
spvDiagnosticDestroy(diagnostic);
spvValidatorOptionsDestroy(options);
spvContextDestroy(context);
// By using the same vector as input and output, we save time in the case

View File

@ -44,6 +44,9 @@ Options:
--max-function-args <maximum number arguments allowed per function>
--max-control-flow-nesting-depth <maximum Control Flow nesting depth allowed>
--max-access-chain-indexes <maximum number of indexes allowed to use for Access Chain instructions>
--relax-struct-store Allow store from one struct type to a
different type with compatible layout and
members.
--version Display validator version information.
--target-env {vulkan1.0|spv1.0|spv1.1|spv1.2}
Use Vulkan1.0/SPIR-V1.0/SPIR-V1.1/SPIR-V1.2 validation rules.
@ -109,6 +112,8 @@ int main(int argc, char** argv) {
continue_processing = false;
return_code = 1;
}
} else if (0 == strcmp(cur_arg, "--relax-struct-store")) {
options.SetRelaxStructStore(true);
} else if (0 == cur_arg[1]) {
// Setting a filename of "-" to indicate stdin.
if (!inFile) {