Move usage detection to after all instructions are registered (#2378)
This is required to properly handle uses of forward declared ids. Since forward
declared ids were not being properly covered by the validator this uncovered a
bunch of small issues that needed to be resolved to get tests passing again.
Fixes #2373
diff --git a/source/val/validate.cpp b/source/val/validate.cpp
index 4024f61..b145b19 100644
--- a/source/val/validate.cpp
+++ b/source/val/validate.cpp
@@ -385,7 +385,6 @@
Instruction* inst = const_cast<Instruction*>(&instruction);
vstate->RegisterInstruction(inst);
}
- if (auto error = UpdateIdUse(*vstate, &instruction)) return error;
}
if (!vstate->has_memory_model_specified())
@@ -399,6 +398,19 @@
// Catch undefined forward references before performing further checks.
if (auto error = ValidateForwardDecls(*vstate)) return error;
+ // ID usage needs be handled in its own iteration of the instructions,
+ // between the two others. It depends on the first loop to have been
+ // finished, so that all instructions have been registered. And the following
+ // loop depends on all of the usage data being populated. Thus it cannot live
+ // in either of those iterations.
+ // It should also live after the forward declaration check, since it will
+ // have problems with missing forward declarations, but give less useful error
+ // messages.
+ for (size_t i = 0; i < vstate->ordered_instructions().size(); ++i) {
+ auto& instruction = vstate->ordered_instructions()[i];
+ if (auto error = UpdateIdUse(*vstate, &instruction)) return error;
+ }
+
// Validate individual opcodes.
for (size_t i = 0; i < vstate->ordered_instructions().size(); ++i) {
auto& instruction = vstate->ordered_instructions()[i];
diff --git a/source/val/validate_decorations.cpp b/source/val/validate_decorations.cpp
index f0ad177..cccf691 100644
--- a/source/val/validate_decorations.cpp
+++ b/source/val/validate_decorations.cpp
@@ -1185,6 +1185,9 @@
// Validates Object operand of an OpStore
for (const auto& use : inst.uses()) {
const auto store = use.first;
+ if (store->opcode() == SpvOpFConvert) continue;
+ if (spvOpcodeIsDebug(store->opcode())) continue;
+ if (spvOpcodeIsDecoration(store->opcode())) continue;
if (store->opcode() != SpvOpStore) {
return vstate.diag(SPV_ERROR_INVALID_ID, &inst)
<< "FPRoundingMode decoration can be applied only to the "
diff --git a/source/val/validate_function.cpp b/source/val/validate_function.cpp
index 96c4776..2f485ce 100644
--- a/source/val/validate_function.cpp
+++ b/source/val/validate_function.cpp
@@ -41,18 +41,22 @@
<< _.getIdName(return_id) << "'.";
}
+ const std::vector<SpvOp> acceptable = {
+ SpvOpDecorate,
+ SpvOpEnqueueKernel,
+ SpvOpEntryPoint,
+ SpvOpExecutionMode,
+ SpvOpExecutionModeId,
+ SpvOpFunctionCall,
+ SpvOpGetKernelNDrangeSubGroupCount,
+ SpvOpGetKernelNDrangeMaxSubGroupSize,
+ SpvOpGetKernelWorkGroupSize,
+ SpvOpGetKernelPreferredWorkGroupSizeMultiple,
+ SpvOpGetKernelLocalSizeForSubgroupCount,
+ SpvOpGetKernelMaxNumSubgroups,
+ SpvOpName};
for (auto& pair : inst->uses()) {
const auto* use = pair.first;
- const std::vector<SpvOp> acceptable = {
- SpvOpFunctionCall,
- SpvOpEntryPoint,
- SpvOpEnqueueKernel,
- SpvOpGetKernelNDrangeSubGroupCount,
- SpvOpGetKernelNDrangeMaxSubGroupSize,
- SpvOpGetKernelWorkGroupSize,
- SpvOpGetKernelPreferredWorkGroupSizeMultiple,
- SpvOpGetKernelLocalSizeForSubgroupCount,
- SpvOpGetKernelMaxNumSubgroups};
if (std::find(acceptable.begin(), acceptable.end(), use->opcode()) ==
acceptable.end()) {
return _.diag(SPV_ERROR_INVALID_ID, use)
diff --git a/source/val/validate_type.cpp b/source/val/validate_type.cpp
index 8e027e4..6246ad5 100644
--- a/source/val/validate_type.cpp
+++ b/source/val/validate_type.cpp
@@ -324,10 +324,12 @@
<< num_args << " arguments.";
}
- // The only valid uses of OpTypeFunction are in an OpFunction instruction.
+ // The only valid uses of OpTypeFunction are in an OpFunction, debugging, or
+ // decoration instruction.
for (auto& pair : inst->uses()) {
const auto* use = pair.first;
- if (use->opcode() != SpvOpFunction) {
+ if (use->opcode() != SpvOpFunction && !spvOpcodeIsDebug(use->opcode()) &&
+ !spvOpcodeIsDecoration(use->opcode())) {
return _.diag(SPV_ERROR_INVALID_ID, use)
<< "Invalid use of function type result id "
<< _.getIdName(inst->id()) << ".";
