Remove spvOpcodeIsObject().

Also
- Add type_id to spv_id_info_t.
- Use spv_id_info_t::type_id instead of words[1].
  Triggered some asserts on tests, where the code incorrectly assumed
  words[1] had a type.  Remove the asserts and handle gracefully.
- Add tests for OpStore of a label, a void, and a function.

diff --git a/source/opcode.cpp b/source/opcode.cpp
index 2fbe81e..fec64a1 100644 (file)
--- a/source/opcode.cpp
+++ b/source/opcode.cpp
@@ -489,121 +489,6 @@ int32_t spvOpcodeIsPointer(const SpvOp opcode) {
   }
 }
 
-int32_t spvOpcodeIsObject(const SpvOp opcode) {
-  switch (opcode) {
-    case SpvOpConstantTrue:
-    case SpvOpConstantFalse:
-    case SpvOpConstant:
-    case SpvOpConstantComposite:
-    // TODO: case SpvOpConstantSampler:
-    case SpvOpConstantNull:
-    case SpvOpSpecConstantTrue:
-    case SpvOpSpecConstantFalse:
-    case SpvOpSpecConstant:
-    case SpvOpSpecConstantComposite:
-    // TODO: case SpvOpSpecConstantOp:
-    case SpvOpVariable:
-    case SpvOpAccessChain:
-    case SpvOpInBoundsAccessChain:
-    case SpvOpConvertFToU:
-    case SpvOpConvertFToS:
-    case SpvOpConvertSToF:
-    case SpvOpConvertUToF:
-    case SpvOpUConvert:
-    case SpvOpSConvert:
-    case SpvOpFConvert:
-    case SpvOpConvertPtrToU:
-    // TODO: case SpvOpConvertUToPtr:
-    case SpvOpPtrCastToGeneric:
-    // TODO: case SpvOpGenericCastToPtr:
-    case SpvOpBitcast:
-    // TODO: case SpvOpGenericCastToPtrExplicit:
-    case SpvOpSatConvertSToU:
-    case SpvOpSatConvertUToS:
-    case SpvOpVectorExtractDynamic:
-    case SpvOpCompositeConstruct:
-    case SpvOpCompositeExtract:
-    case SpvOpCopyObject:
-    case SpvOpTranspose:
-    case SpvOpSNegate:
-    case SpvOpFNegate:
-    case SpvOpNot:
-    case SpvOpIAdd:
-    case SpvOpFAdd:
-    case SpvOpISub:
-    case SpvOpFSub:
-    case SpvOpIMul:
-    case SpvOpFMul:
-    case SpvOpUDiv:
-    case SpvOpSDiv:
-    case SpvOpFDiv:
-    case SpvOpUMod:
-    case SpvOpSRem:
-    case SpvOpSMod:
-    case SpvOpVectorTimesScalar:
-    case SpvOpMatrixTimesScalar:
-    case SpvOpVectorTimesMatrix:
-    case SpvOpMatrixTimesVector:
-    case SpvOpMatrixTimesMatrix:
-    case SpvOpOuterProduct:
-    case SpvOpDot:
-    case SpvOpShiftRightLogical:
-    case SpvOpShiftRightArithmetic:
-    case SpvOpShiftLeftLogical:
-    case SpvOpBitwiseOr:
-    case SpvOpBitwiseXor:
-    case SpvOpBitwiseAnd:
-    case SpvOpAny:
-    case SpvOpAll:
-    case SpvOpIsNan:
-    case SpvOpIsInf:
-    case SpvOpIsFinite:
-    case SpvOpIsNormal:
-    case SpvOpSignBitSet:
-    case SpvOpLessOrGreater:
-    case SpvOpOrdered:
-    case SpvOpUnordered:
-    case SpvOpLogicalOr:
-    case SpvOpLogicalAnd:
-    case SpvOpSelect:
-    case SpvOpIEqual:
-    case SpvOpFOrdEqual:
-    case SpvOpFUnordEqual:
-    case SpvOpINotEqual:
-    case SpvOpFOrdNotEqual:
-    case SpvOpFUnordNotEqual:
-    case SpvOpULessThan:
-    case SpvOpSLessThan:
-    case SpvOpFOrdLessThan:
-    case SpvOpFUnordLessThan:
-    case SpvOpUGreaterThan:
-    case SpvOpSGreaterThan:
-    case SpvOpFOrdGreaterThan:
-    case SpvOpFUnordGreaterThan:
-    case SpvOpULessThanEqual:
-    case SpvOpSLessThanEqual:
-    case SpvOpFOrdLessThanEqual:
-    case SpvOpFUnordLessThanEqual:
-    case SpvOpUGreaterThanEqual:
-    case SpvOpSGreaterThanEqual:
-    case SpvOpFOrdGreaterThanEqual:
-    case SpvOpFUnordGreaterThanEqual:
-    case SpvOpDPdx:
-    case SpvOpDPdy:
-    case SpvOpFwidth:
-    case SpvOpDPdxFine:
-    case SpvOpDPdyFine:
-    case SpvOpFwidthFine:
-    case SpvOpDPdxCoarse:
-    case SpvOpDPdyCoarse:
-    case SpvOpFwidthCoarse:
-    case SpvOpReturnValue:
-      return true;
-    default:
-      return false;
-  }
-}
-
 int32_t spvOpcodeIsBasicTypeNullable(SpvOp opcode) {
   switch (opcode) {
     case SpvOpTypeBool:

diff --git a/source/opcode.h b/source/opcode.h
index 1088863..7ba2b96 100644 (file)
--- a/source/opcode.h
+++ b/source/opcode.h
@@ -92,10 +92,6 @@ int32_t spvOpcodeAreTypesEqual(const spv_instruction_t* type_inst0,
 // non-zero otherwise.
 int32_t spvOpcodeIsPointer(const SpvOp opcode);
 
-// Determines if the given opcode results in an instantation of a non-void type.
-// Returns zero if false, non-zero otherwise.
-int32_t spvOpcodeIsObject(const SpvOp opcode);
-
 // Determines if the scalar type opcode is nullable. Returns zero if false,
 // non-zero otherwise.
 int32_t spvOpcodeIsBasicTypeNullable(SpvOp opcode);

diff --git a/source/validate.cpp b/source/validate.cpp
index a09eb12..f2b5f26 100644 (file)
--- a/source/validate.cpp
+++ b/source/validate.cpp
@@ -274,7 +274,7 @@ void DebugInstructionPass(ValidationState_t& _,
 void ProcessIds(ValidationState_t& _, const spv_parsed_instruction_t& inst) {
   if (inst.result_id) {
     _.usedefs().AddDef(
-        {inst.result_id, inst.opcode,
+        {inst.result_id, inst.type_id, inst.opcode,
          std::vector<uint32_t>(inst.words, inst.words + inst.num_words)});
   }
   for (auto op = inst.operands; op != inst.operands + inst.num_operands; ++op) {

diff --git a/source/validate.h b/source/validate.h
index d0c053e..b94fed2 100644 (file)
--- a/source/validate.h
+++ b/source/validate.h
@@ -47,6 +47,8 @@
 typedef struct spv_id_info_t {
   // Id value.
   uint32_t id;
+  // Type id, or 0 if no type.
+  uint32_t type_id;
   // Opcode of the instruction defining the id.
   SpvOp opcode;
   // Binary words of the instruction defining the id.

diff --git a/source/validate_id.cpp b/source/validate_id.cpp
index 9c42586..47eba66 100644 (file)
--- a/source/validate_id.cpp
+++ b/source/validate_id.cpp
@@ -197,7 +197,7 @@ bool idUsage::isValid<SpvOpEntryPoint>(const spv_instruction_t* inst,
                           << "'s function parameter count is not zero.";
     return false;
   }
-  auto returnType = usedefs_.FindDef(entryPoint.second.words[1]);
+  auto returnType = usedefs_.FindDef(entryPoint.second.type_id);
   if (!returnType.first || SpvOpTypeVoid != returnType.second.opcode) {
     DIAG(entryPointIndex) << "OpEntryPoint Entry Point <id> '"
                           << inst->words[entryPointIndex]
@@ -454,7 +454,7 @@ bool idUsage::isValid<SpvOpConstantComposite>(const spv_instruction_t* inst,
           return false;
         }
         auto constituentResultType =
-            usedefs_.FindDef(constituent.second.words[1]);
+            usedefs_.FindDef(constituent.second.type_id);
         if (!constituentResultType.first ||
             componentType.second.opcode !=
                 constituentResultType.second.opcode) {
@@ -494,7 +494,7 @@ bool idUsage::isValid<SpvOpConstantComposite>(const spv_instruction_t* inst,
                                  << "' is not a constant composite.";
           return false;
         }
-        auto vector = usedefs_.FindDef(constituent.second.words[1]);
+        auto vector = usedefs_.FindDef(constituent.second.type_id);
         assert(vector.first);
         spvCheck(columnType.second.opcode != vector.second.opcode,
                  DIAG(constituentIndex)
@@ -544,7 +544,7 @@ bool idUsage::isValid<SpvOpConstantComposite>(const spv_instruction_t* inst,
                                  << "' is not a constant.";
           return false;
         }
-        auto constituentType = usedefs_.FindDef(constituent.second.words[1]);
+        auto constituentType = usedefs_.FindDef(constituent.second.type_id);
         assert(constituentType.first);
         spvCheck(elementType.second.id != constituentType.second.id,
                  DIAG(constituentIndex)
@@ -575,7 +575,7 @@ bool idUsage::isValid<SpvOpConstantComposite>(const spv_instruction_t* inst,
                                  << "' is not a constant.";
           return false;
         }
-        auto constituentType = usedefs_.FindDef(constituent.second.words[1]);
+        auto constituentType = usedefs_.FindDef(constituent.second.type_id);
         assert(constituentType.first);
 
         auto memberType =
@@ -751,7 +751,7 @@ bool idUsage::isValid<SpvOpLoad>(const spv_instruction_t* inst,
                        << "' is not a pointer.";
     return false;
   }
-  auto pointerType = usedefs_.FindDef(pointer.second.words[1]);
+  auto pointerType = usedefs_.FindDef(pointer.second.type_id);
   if (!pointerType.first || pointerType.second.opcode != SpvOpTypePointer) {
     DIAG(pointerIndex) << "OpLoad type for pointer <id> '"
                        << inst->words[pointerIndex]
@@ -779,7 +779,7 @@ bool idUsage::isValid<SpvOpStore>(const spv_instruction_t* inst,
                        << "' is not a pointer.";
     return false;
   }
-  auto pointerType = usedefs_.FindDef(pointer.second.words[1]);
+  auto pointerType = usedefs_.FindDef(pointer.second.type_id);
   assert(pointerType.first);
   auto type = usedefs_.FindDef(pointerType.second.words[3]);
   assert(type.first);
@@ -791,12 +791,12 @@ bool idUsage::isValid<SpvOpStore>(const spv_instruction_t* inst,
 
   auto objectIndex = 2;
   auto object = usedefs_.FindDef(inst->words[objectIndex]);
-  if (!object.first || !spvOpcodeIsObject(object.second.opcode)) {
+  if (!object.first || !object.second.type_id) {
     DIAG(objectIndex) << "OpStore Object <id> '" << inst->words[objectIndex]
-                      << "' in not an object.";
+                      << "' is not an object.";
     return false;
   }
-  auto objectType = usedefs_.FindDef(object.second.words[1]);
+  auto objectType = usedefs_.FindDef(object.second.type_id);
   assert(objectType.first);
   spvCheck(SpvOpTypeVoid == objectType.second.opcode,
            DIAG(objectIndex) << "OpStore Object <id> '"
@@ -821,11 +821,11 @@ bool idUsage::isValid<SpvOpCopyMemory>(const spv_instruction_t* inst,
   auto sourceIndex = 2;
   auto source = usedefs_.FindDef(inst->words[sourceIndex]);
   if (!source.first) return false;
-  auto targetPointerType = usedefs_.FindDef(target.second.words[1]);
+  auto targetPointerType = usedefs_.FindDef(target.second.type_id);
   assert(targetPointerType.first);
   auto targetType = usedefs_.FindDef(targetPointerType.second.words[3]);
   assert(targetType.first);
-  auto sourcePointerType = usedefs_.FindDef(source.second.words[1]);
+  auto sourcePointerType = usedefs_.FindDef(source.second.type_id);
   assert(sourcePointerType.first);
   auto sourceType = usedefs_.FindDef(sourcePointerType.second.words[3]);
   assert(sourceType.first);
@@ -850,16 +850,16 @@ bool idUsage::isValid<SpvOpCopyMemorySized>(const spv_instruction_t* inst,
   auto sizeIndex = 3;
   auto size = usedefs_.FindDef(inst->words[sizeIndex]);
   if (!size.first) return false;
-  auto targetPointerType = usedefs_.FindDef(target.second.words[1]);
-  assert(targetPointerType.first);
-  spvCheck(SpvOpTypePointer != targetPointerType.second.opcode,
+  auto targetPointerType = usedefs_.FindDef(target.second.type_id);
+  spvCheck(!targetPointerType.first ||
+               SpvOpTypePointer != targetPointerType.second.opcode,
            DIAG(targetIndex) << "OpCopyMemorySized Target <id> '"
                              << inst->words[targetIndex]
                              << "' is not a pointer.";
            return false);
-  auto sourcePointerType = usedefs_.FindDef(source.second.words[1]);
-  assert(sourcePointerType.first);
-  spvCheck(SpvOpTypePointer != sourcePointerType.second.opcode,
+  auto sourcePointerType = usedefs_.FindDef(source.second.type_id);
+  spvCheck(!sourcePointerType.first ||
+               SpvOpTypePointer != sourcePointerType.second.opcode,
            DIAG(sourceIndex) << "OpCopyMemorySized Source <id> '"
                              << inst->words[sourceIndex]
                              << "' is not a pointer.";
@@ -870,7 +870,7 @@ bool idUsage::isValid<SpvOpCopyMemorySized>(const spv_instruction_t* inst,
     // clarification
     case SpvOpConstant:
     case SpvOpSpecConstant: {
-      auto sizeType = usedefs_.FindDef(size.second.words[1]);
+      auto sizeType = usedefs_.FindDef(size.second.type_id);
       assert(sizeType.first);
       spvCheck(SpvOpTypeInt != sizeType.second.opcode,
                DIAG(sizeIndex) << "OpCopyMemorySized Size <id> '"
@@ -879,11 +879,10 @@ bool idUsage::isValid<SpvOpCopyMemorySized>(const spv_instruction_t* inst,
                return false);
     } break;
     case SpvOpVariable: {
-      auto pointerType = usedefs_.FindDef(size.second.words[1]);
+      auto pointerType = usedefs_.FindDef(size.second.type_id);
       assert(pointerType.first);
-      auto sizeType = usedefs_.FindDef(pointerType.second.words[1]);
-      assert(sizeType.first);
-      spvCheck(SpvOpTypeInt != sizeType.second.opcode,
+      auto sizeType = usedefs_.FindDef(pointerType.second.type_id);
+      spvCheck(!sizeType.first || SpvOpTypeInt != sizeType.second.opcode,
                DIAG(sizeIndex) << "OpCopyMemorySized Size <id> '"
                                << inst->words[sizeIndex]
                                << "'s variable type is not an integer type.";
@@ -1003,7 +1002,7 @@ bool idUsage::isValid<SpvOpFunctionCall>(const spv_instruction_t* inst,
                         << inst->words[functionIndex] << "' is not a function.";
     return false;
   }
-  auto returnType = usedefs_.FindDef(function.second.words[1]);
+  auto returnType = usedefs_.FindDef(function.second.type_id);
   assert(returnType.first);
   spvCheck(returnType.second.id != resultType.second.id,
            DIAG(resultTypeIndex) << "OpFunctionCall Result Type <id> '"
@@ -1025,7 +1024,7 @@ bool idUsage::isValid<SpvOpFunctionCall>(const spv_instruction_t* inst,
        argumentIndex < inst->words.size(); argumentIndex++, paramIndex++) {
     auto argument = usedefs_.FindDef(inst->words[argumentIndex]);
     if (!argument.first) return false;
-    auto argumentType = usedefs_.FindDef(argument.second.words[1]);
+    auto argumentType = usedefs_.FindDef(argument.second.type_id);
     assert(argumentType.first);
     auto parameterType =
         usedefs_.FindDef(functionType.second.words[paramIndex]);
@@ -1677,7 +1676,7 @@ bool idUsage::isValid<SpvOpReturnValue>(const spv_instruction_t* inst,
                      << "' does not represent a value.";
     return false;
   }
-  auto valueType = usedefs_.FindDef(value.second.words[1]);
+  auto valueType = usedefs_.FindDef(value.second.type_id);
   assert(valueType.first);
   // NOTE: Find OpFunction
   const spv_instruction_t* function = inst - 1;
@@ -2098,8 +2097,8 @@ bool idUsage::isValid(const spv_instruction_t* inst) {
 #define CASE(OpCode) \
   case Spv##OpCode:  \
     return isValid<Spv##OpCode>(inst, opcodeEntry);
-#define TODO(OpCode)                                     \
-  case Spv##OpCode:                                      \
+#define TODO(OpCode) \
+  case Spv##OpCode:  \
     return true;
   switch (inst->opcode) {
     TODO(OpUndef)

diff --git a/test/ValidateID.cpp b/test/ValidateID.cpp
index f653eb4..e7dacff 100644 (file)
--- a/test/ValidateID.cpp
+++ b/test/ValidateID.cpp
@@ -821,6 +821,53 @@ TEST_F(ValidateID, OpStoreTypeBad) {
   CHECK(spirv, SPV_ERROR_INVALID_ID);
 }
 
+TEST_F(ValidateID, OpStoreVoid) {
+  const char* spirv = R"(
+%1 = OpTypeVoid
+%2 = OpTypeInt 32 1
+%3 = OpTypePointer UniformConstant %2
+%4 = OpTypeFunction %1
+%6 = OpVariable %3 UniformConstant
+%7 = OpFunction %1 None %4
+%8 = OpLabel
+%9 = OpFunctionCall %1 %7
+     OpStore %6 %9
+     OpReturn
+     OpFunctionEnd)";
+  CHECK(spirv, SPV_ERROR_INVALID_ID);
+}
+
+TEST_F(ValidateID, OpStoreLabel) {
+  const char* spirv = R"(
+%1 = OpTypeVoid
+%2 = OpTypeInt 32 1
+%3 = OpTypePointer UniformConstant %2
+%4 = OpTypeFunction %1
+%6 = OpVariable %3 UniformConstant
+%7 = OpFunction %1 None %4
+%8 = OpLabel
+     OpStore %6 %8
+     OpReturn
+     OpFunctionEnd)";
+  CHECK(spirv, SPV_ERROR_INVALID_ID);
+}
+
+// TODO: enable when this bug is fixed: https://cvs.khronos.org/bugzilla/show_bug.cgi?id=15404
+TEST_F(ValidateID, DISABLED_OpStoreFunction) {
+  const char* spirv = R"(
+%2 = OpTypeInt 32 1
+%3 = OpTypePointer UniformConstant %2
+%4 = OpTypeFunction %2
+%5 = OpConstant %2 123
+%6 = OpVariable %3 UniformConstant
+%7 = OpFunction %2 None %4
+%8 = OpLabel
+     OpStore %6 %7
+     OpReturnValue %5
+     OpFunctionEnd)";
+  CHECK(spirv, SPV_ERROR_INVALID_ID);
+}
+
 TEST_F(ValidateID, OpCopyMemoryGood) {
   const char* spirv = R"(
  %1 = OpTypeVoid