From 1f3fb506e8c5ba67257e5806dec0ad8cce8543f3 Mon Sep 17 00:00:00 2001 From: David Neto Date: Wed, 14 Sep 2016 11:57:20 -0400 Subject: [PATCH] Fix validator: OpUndef can be member of a constant composite This was enabled in SPIR-V 1.0 Rev 7 Fixes: https://github.com/KhronosGroup/SPIRV-Tools/issues/414 --- source/opcode.cpp | 4 ++ source/opcode.h | 3 ++ source/validate_id.cpp | 22 +++++++---- test/val/ValidateID.cpp | 102 ++++++++++++++++++++++++++++++++++++++++++++---- 4 files changed, 116 insertions(+), 15 deletions(-) diff --git a/source/opcode.cpp b/source/opcode.cpp index aded404..48c47cc 100644 --- a/source/opcode.cpp +++ b/source/opcode.cpp @@ -205,6 +205,10 @@ int32_t spvOpcodeIsConstant(const SpvOp opcode) { } } +bool spvOpcodeIsConstantOrUndef(const SpvOp opcode) { + return opcode == SpvOpUndef || spvOpcodeIsConstant(opcode); +} + int32_t spvOpcodeIsComposite(const SpvOp opcode) { switch (opcode) { case SpvOpTypeVector: diff --git a/source/opcode.h b/source/opcode.h index f2f592f..60d8e15 100644 --- a/source/opcode.h +++ b/source/opcode.h @@ -65,6 +65,9 @@ int32_t spvOpcodeIsScalarType(const SpvOp opcode); // otherwise. int32_t spvOpcodeIsConstant(const SpvOp opcode); +// Returns true if the given opcode is a constant or undef. +bool spvOpcodeIsConstantOrUndef(const SpvOp opcode); + // Determines if the given opcode is a composite type. Returns zero if false, // non-zero otherwise. int32_t spvOpcodeIsComposite(const SpvOp opcode); diff --git a/source/validate_id.cpp b/source/validate_id.cpp index ecc2246..1e10545 100644 --- a/source/validate_id.cpp +++ b/source/validate_id.cpp @@ -464,10 +464,11 @@ bool idUsage::isValid(const spv_instruction_t* inst, for (size_t constituentIndex = 3; constituentIndex < inst->words.size(); constituentIndex++) { auto constituent = module_.FindDef(inst->words[constituentIndex]); - if (!constituent || !spvOpcodeIsConstant(constituent->opcode())) { + if (!constituent || + !spvOpcodeIsConstantOrUndef(constituent->opcode())) { DIAG(constituentIndex) << "OpConstantComposite Constituent '" << inst->words[constituentIndex] - << "' is not a constant."; + << "' is not a constant or undef."; return false; } auto constituentResultType = module_.FindDef(constituent->type_id()); @@ -502,10 +503,14 @@ bool idUsage::isValid(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()) { + 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 '" << inst->words[constituentIndex] - << "' is not a constant composite."; + << "' is not a constant composite or undef."; return false; } auto vector = module_.FindDef(constituent->type_id()); @@ -553,10 +558,11 @@ bool idUsage::isValid(const spv_instruction_t* inst, for (size_t constituentIndex = 3; constituentIndex < inst->words.size(); constituentIndex++) { auto constituent = module_.FindDef(inst->words[constituentIndex]); - if (!constituent || !spvOpcodeIsConstant(constituent->opcode())) { + if (!constituent || + !spvOpcodeIsConstantOrUndef(constituent->opcode())) { DIAG(constituentIndex) << "OpConstantComposite Constituent '" << inst->words[constituentIndex] - << "' is not a constant."; + << "' is not a constant or undef."; return false; } auto constituentType = module_.FindDef(constituent->type_id()); @@ -584,10 +590,10 @@ bool idUsage::isValid(const spv_instruction_t* inst, constituentIndex < inst->words.size(); constituentIndex++, memberIndex++) { auto constituent = module_.FindDef(inst->words[constituentIndex]); - if (!constituent || !spvOpcodeIsConstant(constituent->opcode())) { + if (!constituent || !spvOpcodeIsConstantOrUndef(constituent->opcode())) { DIAG(constituentIndex) << "OpConstantComposite Constituent '" << inst->words[constituentIndex] - << "' is not a constant."; + << "' is not a constant or undef."; return false; } auto constituentType = module_.FindDef(constituent->type_id()); diff --git a/test/val/ValidateID.cpp b/test/val/ValidateID.cpp index 0b929be..727aa95 100644 --- a/test/val/ValidateID.cpp +++ b/test/val/ValidateID.cpp @@ -615,6 +615,15 @@ TEST_F(ValidateID, OpConstantCompositeVectorGood) { %4 = OpConstantComposite %2 %3 %3 %3 %3)"; CHECK(spirv, SPV_SUCCESS); } +TEST_F(ValidateID, OpConstantCompositeVectorWithUndefGood) { + const char* spirv = R"( +%1 = OpTypeFloat 32 +%2 = OpTypeVector %1 4 +%3 = OpConstant %1 3.14 +%9 = OpUndef %1 +%4 = OpConstantComposite %2 %3 %3 %3 %9)"; + CHECK(spirv, SPV_SUCCESS); +} TEST_F(ValidateID, OpConstantCompositeVectorResultTypeBad) { const char* spirv = R"( %1 = OpTypeFloat 32 @@ -623,13 +632,23 @@ TEST_F(ValidateID, OpConstantCompositeVectorResultTypeBad) { %4 = OpConstantComposite %1 %3 %3 %3 %3)"; CHECK(spirv, SPV_ERROR_INVALID_ID); } -TEST_F(ValidateID, OpConstantCompositeVectorConstituentBad) { +TEST_F(ValidateID, OpConstantCompositeVectorConstituentTypeBad) { + const char* spirv = R"( +%1 = OpTypeFloat 32 +%2 = OpTypeVector %1 4 +%4 = OpTypeInt 32 0 +%3 = OpConstant %1 3.14 +%5 = OpConstant %4 42 ; bad type for constant value +%6 = OpConstantComposite %2 %3 %5 %3 %3)"; + CHECK(spirv, SPV_ERROR_INVALID_ID); +} +TEST_F(ValidateID, OpConstantCompositeVectorConstituentUndefTypeBad) { const char* spirv = R"( %1 = OpTypeFloat 32 %2 = OpTypeVector %1 4 %4 = OpTypeInt 32 0 %3 = OpConstant %1 3.14 -%5 = OpConstant %4 42 +%5 = OpUndef %4 ; bad type for undef value %6 = OpConstantComposite %2 %3 %5 %3 %3)"; CHECK(spirv, SPV_ERROR_INVALID_ID); } @@ -647,7 +666,21 @@ TEST_F(ValidateID, OpConstantCompositeMatrixGood) { %10 = OpConstantComposite %3 %6 %7 %8 %9)"; CHECK(spirv, SPV_SUCCESS); } -TEST_F(ValidateID, OpConstantCompositeMatrixConstituentBad) { +TEST_F(ValidateID, OpConstantCompositeMatrixUndefGood) { + const char* spirv = R"( + %1 = OpTypeFloat 32 + %2 = OpTypeVector %1 4 + %3 = OpTypeMatrix %2 4 + %4 = OpConstant %1 1.0 + %5 = OpConstant %1 0.0 + %6 = OpConstantComposite %2 %4 %5 %5 %5 + %7 = OpConstantComposite %2 %5 %4 %5 %5 + %8 = OpConstantComposite %2 %5 %5 %4 %5 + %9 = OpUndef %2 +%10 = OpConstantComposite %3 %6 %7 %8 %9)"; + CHECK(spirv, SPV_SUCCESS); +} +TEST_F(ValidateID, OpConstantCompositeMatrixConstituentTypeBad) { const char* spirv = R"( %1 = OpTypeFloat 32 %2 = OpTypeVector %1 4 @@ -662,6 +695,21 @@ TEST_F(ValidateID, OpConstantCompositeMatrixConstituentBad) { %10 = OpConstantComposite %3 %6 %7 %8 %9)"; CHECK(spirv, SPV_ERROR_INVALID_ID); } +TEST_F(ValidateID, OpConstantCompositeMatrixConstituentUndefTypeBad) { + const char* spirv = R"( + %1 = OpTypeFloat 32 + %2 = OpTypeVector %1 4 +%11 = OpTypeVector %1 3 + %3 = OpTypeMatrix %2 4 + %4 = OpConstant %1 1.0 + %5 = OpConstant %1 0.0 + %6 = OpConstantComposite %2 %4 %5 %5 %5 + %7 = OpConstantComposite %2 %5 %4 %5 %5 + %8 = OpConstantComposite %2 %5 %5 %4 %5 + %9 = OpUndef %11 +%10 = OpConstantComposite %3 %6 %7 %8 %9)"; + CHECK(spirv, SPV_ERROR_INVALID_ID); +} TEST_F(ValidateID, OpConstantCompositeMatrixColumnTypeBad) { const char* spirv = R"( %1 = OpTypeInt 32 0 @@ -684,21 +732,40 @@ TEST_F(ValidateID, OpConstantCompositeArrayGood) { %4 = OpConstantComposite %3 %2 %2 %2 %2)"; CHECK(spirv, SPV_SUCCESS); } +TEST_F(ValidateID, OpConstantCompositeArrayWithUndefGood) { + const char* spirv = R"( +%1 = OpTypeInt 32 0 +%2 = OpConstant %1 4 +%9 = OpUndef %1 +%3 = OpTypeArray %1 %2 +%4 = OpConstantComposite %3 %2 %2 %2 %9)"; + CHECK(spirv, SPV_SUCCESS); +} TEST_F(ValidateID, OpConstantCompositeArrayConstConstituentBad) { const char* spirv = R"( %1 = OpTypeInt 32 0 %2 = OpConstant %1 4 %3 = OpTypeArray %1 %2 -%4 = OpConstantComposite %3 %2 %2 %2 %1)"; +%4 = OpConstantComposite %3 %2 %2 %2 %1)"; // Uses a type as operand + CHECK(spirv, SPV_ERROR_INVALID_ID); +} +TEST_F(ValidateID, OpConstantCompositeArrayConstituentTypeBad) { + const char* spirv = R"( +%1 = OpTypeInt 32 0 +%2 = OpConstant %1 4 +%3 = OpTypeArray %1 %2 +%5 = OpTypeFloat 32 +%6 = OpConstant %5 3.14 ; bad type for const value +%4 = OpConstantComposite %3 %2 %2 %2 %6)"; CHECK(spirv, SPV_ERROR_INVALID_ID); } -TEST_F(ValidateID, OpConstantCompositeArrayConstituentBad) { +TEST_F(ValidateID, OpConstantCompositeArrayConstituentUndefTypeBad) { const char* spirv = R"( %1 = OpTypeInt 32 0 %2 = OpConstant %1 4 %3 = OpTypeArray %1 %2 %5 = OpTypeFloat 32 -%6 = OpConstant %5 3.14 +%6 = OpUndef %5 ; bad type for undef %4 = OpConstantComposite %3 %2 %2 %2 %6)"; CHECK(spirv, SPV_ERROR_INVALID_ID); } @@ -712,7 +779,17 @@ TEST_F(ValidateID, OpConstantCompositeStructGood) { %6 = OpConstantComposite %3 %4 %4 %5)"; CHECK(spirv, SPV_SUCCESS); } -TEST_F(ValidateID, OpConstantCompositeStructMemberBad) { +TEST_F(ValidateID, OpConstantCompositeStructUndefGood) { + const char* spirv = R"( +%1 = OpTypeInt 32 0 +%2 = OpTypeInt 64 1 +%3 = OpTypeStruct %1 %1 %2 +%4 = OpConstant %1 42 +%5 = OpUndef %2 +%6 = OpConstantComposite %3 %4 %4 %5)"; + CHECK(spirv, SPV_SUCCESS); +} +TEST_F(ValidateID, OpConstantCompositeStructMemberTypeBad) { const char* spirv = R"( %1 = OpTypeInt 32 0 %2 = OpTypeInt 64 1 @@ -723,6 +800,17 @@ TEST_F(ValidateID, OpConstantCompositeStructMemberBad) { CHECK(spirv, SPV_ERROR_INVALID_ID); } +TEST_F(ValidateID, OpConstantCompositeStructMemberUndefTypeBad) { + const char* spirv = R"( +%1 = OpTypeInt 32 0 +%2 = OpTypeInt 64 1 +%3 = OpTypeStruct %1 %1 %2 +%4 = OpConstant %1 42 +%5 = OpUndef %2 +%6 = OpConstantComposite %3 %4 %5 %4)"; + CHECK(spirv, SPV_ERROR_INVALID_ID); +} + TEST_F(ValidateID, OpConstantSamplerGood) { const char* spirv = R"( %float = OpTypeFloat 32 -- 2.7.4