From 276a724b2580dc66936b5235f672c03241937cff Mon Sep 17 00:00:00 2001
From: Dejan Mircevski <deki@google.com>
Date: Thu, 21 Jan 2016 15:55:43 -0500
Subject: [PATCH] Fix spvOpcodeIsScalarType() to include Boolean.

Remove redundant validations of OpConstant and OpConstantComposite.
Binary parser already performs these checks, so the validations can
never be triggered.

Enable bad-constant tests.
---
 source/opcode.cpp      |  1 +
 source/validate_id.cpp | 30 ------------------------------
 test/ValidateID.cpp    | 38 +++++++++++++++++++++++++++++++++-----
 3 files changed, 34 insertions(+), 35 deletions(-)

diff --git a/source/opcode.cpp b/source/opcode.cpp
index 526933d1c..2fbe81ec7 100644
--- a/source/opcode.cpp
+++ b/source/opcode.cpp
@@ -431,6 +431,7 @@ int32_t spvOpcodeIsScalarType(const SpvOp opcode) {
   switch (opcode) {
     case SpvOpTypeInt:
     case SpvOpTypeFloat:
+    case SpvOpTypeBool:
       return true;
     default:
       return false;
diff --git a/source/validate_id.cpp b/source/validate_id.cpp
index e209b32a9..9c42586bb 100644
--- a/source/validate_id.cpp
+++ b/source/validate_id.cpp
@@ -417,20 +417,6 @@ bool idUsage::isValid<SpvOpConstantFalse>(const spv_instruction_t* inst,
   return true;
 }
 
-template <>
-bool idUsage::isValid<SpvOpConstant>(const spv_instruction_t* inst,
-                                     const spv_opcode_desc) {
-  auto resultTypeIndex = 1;
-  auto resultType = usedefs_.FindDef(inst->words[resultTypeIndex]);
-  if (!resultType.first || !spvOpcodeIsScalarType(resultType.second.opcode)) {
-    DIAG(resultTypeIndex)
-        << "OpConstant Result Type <id> '" << inst->words[resultTypeIndex]
-        << "' is not a scalar integer or floating point type.";
-    return false;
-  }
-  return true;
-}
-
 template <>
 bool idUsage::isValid<SpvOpConstantComposite>(const spv_instruction_t* inst,
                                               const spv_opcode_desc) {
@@ -713,20 +699,6 @@ bool idUsage::isValid<SpvOpSpecConstantFalse>(const spv_instruction_t* inst,
   return true;
 }
 
-template <>
-bool idUsage::isValid<SpvOpSpecConstant>(const spv_instruction_t* inst,
-                                         const spv_opcode_desc) {
-  auto resultTypeIndex = 1;
-  auto resultType = usedefs_.FindDef(inst->words[resultTypeIndex]);
-  if (!resultType.first || !spvOpcodeIsScalarType(resultType.second.opcode)) {
-    DIAG(resultTypeIndex) << "OpSpecConstant Result Type <id> '"
-                          << inst->words[resultTypeIndex]
-                          << "' is not a scalar type.";
-    return false;
-  }
-  return true;
-}
-
 #if 0
 template <>
 bool idUsage::isValid<SpvOpSpecConstantComposite>(
@@ -2150,13 +2122,11 @@ bool idUsage::isValid(const spv_instruction_t* inst) {
     CASE(OpTypePipe)
     CASE(OpConstantTrue)
     CASE(OpConstantFalse)
-    CASE(OpConstant)
     CASE(OpConstantComposite)
     CASE(OpConstantSampler)
     CASE(OpConstantNull)
     CASE(OpSpecConstantTrue)
     CASE(OpSpecConstantFalse)
-    CASE(OpSpecConstant)
     TODO(OpSpecConstantComposite)
     TODO(OpSpecConstantOp)
     CASE(OpVariable)
diff --git a/test/ValidateID.cpp b/test/ValidateID.cpp
index e1a1d924d..f653eb447 100644
--- a/test/ValidateID.cpp
+++ b/test/ValidateID.cpp
@@ -257,12 +257,34 @@ TEST_F(ValidateID, OpExecutionModeEntryPointBad) {
   CHECK(spirv, SPV_ERROR_INVALID_ID);
 }
 
-TEST_F(ValidateID, OpTypeVectorGood) {
+TEST_F(ValidateID, OpTypeVectorFloat) {
   const char* spirv = R"(
 %1 = OpTypeFloat 32
 %2 = OpTypeVector %1 4)";
   CHECK(spirv, SPV_SUCCESS);
 }
+
+TEST_F(ValidateID, OpTypeVectorInt) {
+  const char* spirv = R"(
+%1 = OpTypeInt 32 1
+%2 = OpTypeVector %1 4)";
+  CHECK(spirv, SPV_SUCCESS);
+}
+
+TEST_F(ValidateID, OpTypeVectorUInt) {
+  const char* spirv = R"(
+%1 = OpTypeInt 64 0
+%2 = OpTypeVector %1 4)";
+  CHECK(spirv, SPV_SUCCESS);
+}
+
+TEST_F(ValidateID, OpTypeVectorBool) {
+  const char* spirv = R"(
+%1 = OpTypeBool
+%2 = OpTypeVector %1 4)";
+  CHECK(spirv, SPV_SUCCESS);
+}
+
 TEST_F(ValidateID, OpTypeVectorComponentTypeBad) {
   const char* spirv = R"(
 %1 = OpTypeFloat 32
@@ -423,11 +445,14 @@ TEST_F(ValidateID, OpConstantGood) {
 %2 = OpConstant %1 1)";
   CHECK(spirv, SPV_SUCCESS);
 }
-TEST_F(ValidateID, DISABLED_OpConstantBad) {
+TEST_F(ValidateID, OpConstantBad) {
   const char* spirv = R"(
 %1 = OpTypeVoid
 %2 = OpConstant !1 !0)";
-  CHECK(spirv, SPV_ERROR_INVALID_ID);
+  // The expected failure code is implementation dependent (currently
+  // INVALID_BINARY because the binary parser catches these cases) and may
+  // change over time, but this must always fail.
+  CHECK(spirv, SPV_ERROR_INVALID_BINARY);
 }
 
 TEST_F(ValidateID, OpConstantCompositeVectorGood) {
@@ -645,11 +670,14 @@ TEST_F(ValidateID, OpSpecConstantGood) {
 %2 = OpSpecConstant %1 42)";
   CHECK(spirv, SPV_SUCCESS);
 }
-TEST_F(ValidateID, DISABLED_OpSpecConstantBad) {
+TEST_F(ValidateID, OpSpecConstantBad) {
   const char* spirv = R"(
 %1 = OpTypeVoid
 %2 = OpSpecConstant !1 !4)";
-  CHECK(spirv, SPV_ERROR_INVALID_ID);
+  // The expected failure code is implementation dependent (currently
+  // INVALID_BINARY because the binary parser catches these cases) and may
+  // change over time, but this must always fail.
+  CHECK(spirv, SPV_ERROR_INVALID_BINARY);
 }
 
 // TODO: OpSpecConstantComposite