Split annotation opcode validation into new file.
* Moves annotation opcode checks from idUsage into a new pass
* minor style updates
diff --git a/Android.mk b/Android.mk
index 05fece5..8220a0e 100644
--- a/Android.mk
+++ b/Android.mk
@@ -36,6 +36,7 @@
source/val/validation_state.cpp \
source/val/validate.cpp \
source/val/validate_adjacency.cpp \
+ source/val/validate_annotation.cpp \
source/val/validate_arithmetics.cpp \
source/val/validate_atomics.cpp \
source/val/validate_barriers.cpp \
diff --git a/BUILD.gn b/BUILD.gn
index 6ad8f7d..5b0ec6a 100644
--- a/BUILD.gn
+++ b/BUILD.gn
@@ -359,6 +359,7 @@
"source/val/validate.cpp",
"source/val/validate.h",
"source/val/validate_adjacency.cpp",
+ "source/val/validate_annotation.cpp",
"source/val/validate_arithmetics.cpp",
"source/val/validate_atomics.cpp",
"source/val/validate_barriers.cpp",
diff --git a/source/CMakeLists.txt b/source/CMakeLists.txt
index ce3bcaa..3b8bd93 100644
--- a/source/CMakeLists.txt
+++ b/source/CMakeLists.txt
@@ -283,6 +283,7 @@
${CMAKE_CURRENT_SOURCE_DIR}/text_handler.cpp
${CMAKE_CURRENT_SOURCE_DIR}/val/validate.cpp
${CMAKE_CURRENT_SOURCE_DIR}/val/validate_adjacency.cpp
+ ${CMAKE_CURRENT_SOURCE_DIR}/val/validate_annotation.cpp
${CMAKE_CURRENT_SOURCE_DIR}/val/validate_arithmetics.cpp
${CMAKE_CURRENT_SOURCE_DIR}/val/validate_atomics.cpp
${CMAKE_CURRENT_SOURCE_DIR}/val/validate_barriers.cpp
diff --git a/source/val/validate.cpp b/source/val/validate.cpp
index f9ac9a2..c26f00f 100644
--- a/source/val/validate.cpp
+++ b/source/val/validate.cpp
@@ -322,7 +322,7 @@
// sections to maintain test consistency.
// Miscellaneous
if (auto error = DebugPass(*vstate, &instruction)) return error;
- // Annotation
+ if (auto error = AnnotationPass(*vstate, &instruction)) return error;
if (auto error = ExtInstPass(*vstate, &instruction)) return error;
// Mode Setting
if (auto error = TypePass(*vstate, &instruction)) return error;
diff --git a/source/val/validate.h b/source/val/validate.h
index d8e8a0c..e59d8b1 100644
--- a/source/val/validate.h
+++ b/source/val/validate.h
@@ -172,6 +172,9 @@
/// Validates correctness of ExtInst instructions.
spv_result_t ExtInstPass(ValidationState_t& _, const Instruction* inst);
+/// Validates correctness of annotation instructions.
+spv_result_t AnnotationPass(ValidationState_t& _, const Instruction* inst);
+
/// Validates correctness of non-uniform group instructions.
spv_result_t NonUniformPass(ValidationState_t& _, const Instruction* inst);
diff --git a/source/val/validate_annotation.cpp b/source/val/validate_annotation.cpp
new file mode 100644
index 0000000..f175839
--- /dev/null
+++ b/source/val/validate_annotation.cpp
@@ -0,0 +1,158 @@
+// Copyright (c) 2018 Google LLC.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+#include "source/val/validate.h"
+
+#include "source/opcode.h"
+#include "source/val/instruction.h"
+#include "source/val/validation_state.h"
+
+namespace spvtools {
+namespace val {
+namespace {
+
+spv_result_t ValidateDecorate(ValidationState_t& _, const Instruction* inst) {
+ const auto decoration = inst->GetOperandAs<uint32_t>(1);
+ if (decoration == SpvDecorationSpecId) {
+ const auto target_id = inst->GetOperandAs<uint32_t>(0);
+ const auto target = _.FindDef(target_id);
+ if (!target || !spvOpcodeIsScalarSpecConstant(target->opcode())) {
+ return _.diag(SPV_ERROR_INVALID_ID, inst)
+ << "OpDecorate SpecId decoration target <id> '"
+ << _.getIdName(decoration)
+ << "' is not a scalar specialization constant.";
+ }
+ }
+ // TODO: Add validations for all decorations.
+ return SPV_SUCCESS;
+}
+
+spv_result_t ValidateMemberDecorate(ValidationState_t& _,
+ const Instruction* inst) {
+ const auto struct_type_id = inst->GetOperandAs<uint32_t>(0);
+ const auto struct_type = _.FindDef(struct_type_id);
+ if (!struct_type || SpvOpTypeStruct != struct_type->opcode()) {
+ return _.diag(SPV_ERROR_INVALID_ID, inst)
+ << "OpMemberDecorate Structure type <id> '"
+ << _.getIdName(struct_type_id) << "' is not a struct type.";
+ }
+ const auto member = inst->GetOperandAs<uint32_t>(1);
+ const auto member_count =
+ static_cast<uint32_t>(struct_type->words().size() - 2);
+ if (member_count < member) {
+ return _.diag(SPV_ERROR_INVALID_ID, inst)
+ << "Index " << member
+ << " provided in OpMemberDecorate for struct <id> "
+ << _.getIdName(struct_type_id)
+ << " is out of bounds. The structure has " << member_count
+ << " members. Largest valid index is " << member_count - 1 << ".";
+ }
+ return SPV_SUCCESS;
+}
+
+spv_result_t ValidateDecorationGroup(ValidationState_t& _,
+ const Instruction* inst) {
+ const auto decoration_group_id = inst->GetOperandAs<uint32_t>(0);
+ const auto decoration_group = _.FindDef(decoration_group_id);
+ for (auto pair : decoration_group->uses()) {
+ auto use = pair.first;
+ if (use->opcode() != SpvOpDecorate && use->opcode() != SpvOpGroupDecorate &&
+ use->opcode() != SpvOpGroupMemberDecorate &&
+ use->opcode() != SpvOpName) {
+ return _.diag(SPV_ERROR_INVALID_ID, inst)
+ << "Result id of OpDecorationGroup can only "
+ << "be targeted by OpName, OpGroupDecorate, "
+ << "OpDecorate, and OpGroupMemberDecorate";
+ }
+ }
+ return SPV_SUCCESS;
+}
+
+spv_result_t ValidateGroupDecorate(ValidationState_t& _,
+ const Instruction* inst) {
+ const auto decoration_group_id = inst->GetOperandAs<uint32_t>(0);
+ auto decoration_group = _.FindDef(decoration_group_id);
+ if (!decoration_group || SpvOpDecorationGroup != decoration_group->opcode()) {
+ return _.diag(SPV_ERROR_INVALID_ID, inst)
+ << "OpGroupDecorate Decoration group <id> '"
+ << _.getIdName(decoration_group_id)
+ << "' is not a decoration group.";
+ }
+ return SPV_SUCCESS;
+}
+
+spv_result_t ValidateGroupMemberDecorate(ValidationState_t& _,
+ const Instruction* inst) {
+ const auto decoration_group_id = inst->GetOperandAs<uint32_t>(0);
+ const auto decoration_group = _.FindDef(decoration_group_id);
+ if (!decoration_group || SpvOpDecorationGroup != decoration_group->opcode()) {
+ return _.diag(SPV_ERROR_INVALID_ID, inst)
+ << "OpGroupMemberDecorate Decoration group <id> '"
+ << _.getIdName(decoration_group_id)
+ << "' is not a decoration group.";
+ }
+ // Grammar checks ensures that the number of arguments to this instruction
+ // is an odd number: 1 decoration group + (id,literal) pairs.
+ for (size_t i = 1; i + 1 < inst->operands().size(); i += 2) {
+ const uint32_t struct_id = inst->GetOperandAs<uint32_t>(i);
+ const uint32_t index = inst->GetOperandAs<uint32_t>(i + 1);
+ auto struct_instr = _.FindDef(struct_id);
+ if (!struct_instr || SpvOpTypeStruct != struct_instr->opcode()) {
+ return _.diag(SPV_ERROR_INVALID_ID, inst)
+ << "OpGroupMemberDecorate Structure type <id> '"
+ << _.getIdName(struct_id) << "' is not a struct type.";
+ }
+ const uint32_t num_struct_members =
+ static_cast<uint32_t>(struct_instr->words().size() - 2);
+ if (index >= num_struct_members) {
+ return _.diag(SPV_ERROR_INVALID_ID, inst)
+ << "Index " << index
+ << " provided in OpGroupMemberDecorate for struct <id> "
+ << _.getIdName(struct_id)
+ << " is out of bounds. The structure has " << num_struct_members
+ << " members. Largest valid index is " << num_struct_members - 1
+ << ".";
+ }
+ }
+ return SPV_SUCCESS;
+}
+
+} // namespace
+
+spv_result_t AnnotationPass(ValidationState_t& _, const Instruction* inst) {
+ switch (inst->opcode()) {
+ case SpvOpDecorate:
+ if (auto error = ValidateDecorate(_, inst)) return error;
+ break;
+ case SpvOpMemberDecorate:
+ if (auto error = ValidateMemberDecorate(_, inst)) return error;
+ break;
+ case SpvOpDecorationGroup:
+ if (auto error = ValidateDecorationGroup(_, inst)) return error;
+ break;
+ case SpvOpGroupDecorate:
+ if (auto error = ValidateGroupDecorate(_, inst)) return error;
+ break;
+ case SpvOpGroupMemberDecorate:
+ if (auto error = ValidateGroupMemberDecorate(_, inst)) return error;
+ break;
+ default:
+ break;
+ }
+
+ return SPV_SUCCESS;
+}
+
+} // namespace val
+} // namespace spvtools
diff --git a/source/val/validate_id.cpp b/source/val/validate_id.cpp
index bed709a..415192f 100644
--- a/source/val/validate_id.cpp
+++ b/source/val/validate_id.cpp
@@ -92,126 +92,6 @@
helper
template <>
-bool idUsage::isValid<SpvOpDecorate>(const spv_instruction_t* inst,
- const spv_opcode_desc) {
- auto decorationIndex = 2;
- auto decoration = inst->words[decorationIndex];
- if (decoration == SpvDecorationSpecId) {
- auto targetIndex = 1;
- auto target = module_.FindDef(inst->words[targetIndex]);
- if (!target || !spvOpcodeIsScalarSpecConstant(target->opcode())) {
- DIAG(target) << "OpDecorate SpectId decoration target <id> '"
- << module_.getIdName(inst->words[decorationIndex])
- << "' is not a scalar specialization constant.";
- return false;
- }
- }
- // TODO: Add validations for all decorations.
- return true;
-}
-
-template <>
-bool idUsage::isValid<SpvOpMemberDecorate>(const spv_instruction_t* inst,
- const spv_opcode_desc) {
- auto structTypeIndex = 1;
- auto structType = module_.FindDef(inst->words[structTypeIndex]);
- if (!structType || SpvOpTypeStruct != structType->opcode()) {
- DIAG(structType) << "OpMemberDecorate Structure type <id> '"
- << module_.getIdName(inst->words[structTypeIndex])
- << "' is not a struct type.";
- return false;
- }
- auto memberIndex = 2;
- auto member = inst->words[memberIndex];
- auto memberCount = static_cast<uint32_t>(structType->words().size() - 2);
- if (memberCount < member) {
- DIAG(structType) << "Index " << member
- << " provided in OpMemberDecorate for struct <id> "
- << module_.getIdName(inst->words[structTypeIndex])
- << " is out of bounds. The structure has " << memberCount
- << " members. Largest valid index is " << memberCount - 1
- << ".";
- return false;
- }
- return true;
-}
-
-template <>
-bool idUsage::isValid<SpvOpDecorationGroup>(const spv_instruction_t* inst,
- const spv_opcode_desc) {
- auto decorationGroupIndex = 1;
- auto decorationGroup = module_.FindDef(inst->words[decorationGroupIndex]);
-
- for (auto pair : decorationGroup->uses()) {
- auto use = pair.first;
- if (use->opcode() != SpvOpDecorate && use->opcode() != SpvOpGroupDecorate &&
- use->opcode() != SpvOpGroupMemberDecorate &&
- use->opcode() != SpvOpName) {
- DIAG(decorationGroup) << "Result id of OpDecorationGroup can only "
- << "be targeted by OpName, OpGroupDecorate, "
- << "OpDecorate, and OpGroupMemberDecorate";
- return false;
- }
- }
- return true;
-}
-
-template <>
-bool idUsage::isValid<SpvOpGroupDecorate>(const spv_instruction_t* inst,
- const spv_opcode_desc) {
- auto decorationGroupIndex = 1;
- auto decorationGroup = module_.FindDef(inst->words[decorationGroupIndex]);
- if (!decorationGroup || SpvOpDecorationGroup != decorationGroup->opcode()) {
- DIAG(decorationGroup) << "OpGroupDecorate Decoration group <id> '"
- << module_.getIdName(
- 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) {
- auto decorationGroupIndex = 1;
- auto decorationGroup = module_.FindDef(inst->words[decorationGroupIndex]);
- if (!decorationGroup || SpvOpDecorationGroup != decorationGroup->opcode()) {
- DIAG(decorationGroup) << "OpGroupMemberDecorate Decoration group <id> '"
- << module_.getIdName(
- inst->words[decorationGroupIndex])
- << "' is not a decoration group.";
- return false;
- }
- // Grammar checks ensures that the number of arguments to this instruction
- // is an odd number: 1 decoration group + (id,literal) pairs.
- for (size_t i = 2; i + 1 < inst->words.size(); i = i + 2) {
- const uint32_t struct_id = inst->words[i];
- const uint32_t index = inst->words[i + 1];
- auto struct_instr = module_.FindDef(struct_id);
- if (!struct_instr || SpvOpTypeStruct != struct_instr->opcode()) {
- DIAG(struct_instr) << "OpGroupMemberDecorate Structure type <id> '"
- << module_.getIdName(struct_id)
- << "' is not a struct type.";
- return false;
- }
- const uint32_t num_struct_members =
- static_cast<uint32_t>(struct_instr->words().size() - 2);
- if (index >= num_struct_members) {
- DIAG(struct_instr)
- << "Index " << index
- << " provided in OpGroupMemberDecorate for struct <id> "
- << module_.getIdName(struct_id)
- << " is out of bounds. The structure has " << num_struct_members
- << " members. Largest valid index is " << num_struct_members - 1
- << ".";
- return false;
- }
- }
- return true;
-}
-
-template <>
bool idUsage::isValid<SpvOpEntryPoint>(const spv_instruction_t* inst,
const spv_opcode_desc) {
auto entryPointIndex = 2;
@@ -1144,11 +1024,6 @@
case Spv##OpCode: \
return isValid<Spv##OpCode>(inst, opcodeEntry);
switch (inst->opcode) {
- CASE(OpDecorate)
- CASE(OpMemberDecorate)
- CASE(OpDecorationGroup)
- CASE(OpGroupDecorate)
- CASE(OpGroupMemberDecorate)
CASE(OpEntryPoint)
CASE(OpExecutionMode)
CASE(OpConstantTrue)
diff --git a/test/val/val_id_test.cpp b/test/val/val_id_test.cpp
index e577962..a1bc627 100644
--- a/test/val/val_id_test.cpp
+++ b/test/val/val_id_test.cpp
@@ -4884,10 +4884,9 @@
)";
CompileSuccessfully(spirv.c_str());
EXPECT_EQ(SPV_ERROR_INVALID_ID, ValidateInstructions());
- EXPECT_THAT(
- getDiagnosticString(),
- HasSubstr("OpDecorate SpectId decoration target <id> '1' is not a "
- "scalar specialization constant."));
+ EXPECT_THAT(getDiagnosticString(),
+ HasSubstr("OpDecorate SpecId decoration target <id> '1' is not a "
+ "scalar specialization constant."));
}
TEST_F(ValidateIdWithMessage, SpecIdTargetOpSpecConstantOpBad) {
@@ -4906,10 +4905,9 @@
)";
CompileSuccessfully(spirv.c_str());
EXPECT_EQ(SPV_ERROR_INVALID_ID, ValidateInstructions());
- EXPECT_THAT(
- getDiagnosticString(),
- HasSubstr("OpDecorate SpectId decoration target <id> '1' is not a "
- "scalar specialization constant."));
+ EXPECT_THAT(getDiagnosticString(),
+ HasSubstr("OpDecorate SpecId decoration target <id> '1' is not a "
+ "scalar specialization constant."));
}
TEST_F(ValidateIdWithMessage, SpecIdTargetOpSpecConstantCompositeBad) {
@@ -4927,10 +4925,9 @@
)";
CompileSuccessfully(spirv.c_str());
EXPECT_EQ(SPV_ERROR_INVALID_ID, ValidateInstructions());
- EXPECT_THAT(
- getDiagnosticString(),
- HasSubstr("OpDecorate SpectId decoration target <id> '1' is not a "
- "scalar specialization constant."));
+ EXPECT_THAT(getDiagnosticString(),
+ HasSubstr("OpDecorate SpecId decoration target <id> '1' is not a "
+ "scalar specialization constant."));
}
TEST_F(ValidateIdWithMessage, SpecIdTargetGood) {
