From d5b0cd34c95a5dca4af8e83560af25cc581e8034 Mon Sep 17 00:00:00 2001
From: David Neto <dneto@google.com>
Date: Wed, 7 Dec 2016 10:33:27 -0500
Subject: [PATCH] An OpVariable initializer can be a module-scope variable

Fixes https://github.com/KhronosGroup/SPIRV-Tools/issues/482
---
 source/validate_id.cpp   | 18 ++++++++++------
 test/val/val_id_test.cpp | 56 ++++++++++++++++++++++++++++++++++++++++++++++--
 2 files changed, 66 insertions(+), 8 deletions(-)

diff --git a/source/validate_id.cpp b/source/validate_id.cpp
index 292c8e3..1ebfa82 100644
--- a/source/validate_id.cpp
+++ b/source/validate_id.cpp
@@ -988,7 +988,7 @@ bool idUsage::isValid<SpvOpSpecConstantOp>(const spv_instruction_t *inst) {}
 
 template <>
 bool idUsage::isValid<SpvOpVariable>(const spv_instruction_t* inst,
-                                     const spv_opcode_desc opcodeEntry) {
+                                     const spv_opcode_desc) {
   auto resultTypeIndex = 1;
   auto resultType = module_.FindDef(inst->words[resultTypeIndex]);
   if (!resultType || SpvOpTypePointer != resultType->opcode()) {
@@ -997,13 +997,19 @@ bool idUsage::isValid<SpvOpVariable>(const spv_instruction_t* inst,
                           << "' is not a pointer type.";
     return false;
   }
-  if (opcodeEntry->numTypes < inst->words.size()) {
-    auto initialiserIndex = 4;
-    auto initialiser = module_.FindDef(inst->words[initialiserIndex]);
-    if (!initialiser || !spvOpcodeIsConstant(initialiser->opcode())) {
+  const auto initialiserIndex = 4;
+  if (initialiserIndex < inst->words.size()) {
+    const auto initialiser = module_.FindDef(inst->words[initialiserIndex]);
+    const auto storageClassIndex = 3;
+    const auto is_module_scope_var =
+        initialiser && (initialiser->opcode() == SpvOpVariable) &&
+        (initialiser->word(storageClassIndex) != SpvStorageClassFunction);
+    const auto is_constant =
+        initialiser && spvOpcodeIsConstant(initialiser->opcode());
+    if (!initialiser || !(is_constant || is_module_scope_var)) {
       DIAG(initialiserIndex) << "OpVariable Initializer <id> '"
                              << inst->words[initialiserIndex]
-                             << "' is not a constant.";
+                             << "' is not a constant or module-scope variable.";
       return false;
     }
   }
diff --git a/test/val/val_id_test.cpp b/test/val/val_id_test.cpp
index dd7335a..0e4964e 100644
--- a/test/val/val_id_test.cpp
+++ b/test/val/val_id_test.cpp
@@ -1461,7 +1461,7 @@ TEST_F(ValidateIdWithMessage, OpVariableGood) {
   CompileSuccessfully(spirv.c_str());
   EXPECT_EQ(SPV_SUCCESS, ValidateInstructions());
 }
-TEST_F(ValidateIdWithMessage, OpVariableInitializerGood) {
+TEST_F(ValidateIdWithMessage, OpVariableInitializerConstantGood) {
   string spirv = kGLSL450MemoryModel + R"(
 %1 = OpTypeInt 32 1
 %2 = OpTypePointer Input %1
@@ -1470,6 +1470,16 @@ TEST_F(ValidateIdWithMessage, OpVariableInitializerGood) {
   CompileSuccessfully(spirv.c_str());
   EXPECT_EQ(SPV_SUCCESS, ValidateInstructions());
 }
+TEST_F(ValidateIdWithMessage, OpVariableInitializerGlobalVariableGood) {
+  string spirv = kGLSL450MemoryModel + R"(
+%1 = OpTypeInt 32 1
+%2 = OpTypePointer Uniform %1
+%3 = OpVariable %2 Uniform
+%4 = OpTypePointer Uniform %2 ; pointer to pointer
+%5 = OpVariable %4 Uniform %3)";
+  CompileSuccessfully(spirv.c_str());
+  EXPECT_EQ(SPV_SUCCESS, ValidateInstructions());
+}
 // TODO: Positive test OpVariable with OpConstantNull of OpTypePointer
 TEST_F(ValidateIdWithMessage, OpVariableResultTypeBad) {
   string spirv = kGLSL450MemoryModel + R"(
@@ -1478,13 +1488,55 @@ TEST_F(ValidateIdWithMessage, OpVariableResultTypeBad) {
   CompileSuccessfully(spirv.c_str());
   EXPECT_EQ(SPV_ERROR_INVALID_ID, ValidateInstructions());
 }
-TEST_F(ValidateIdWithMessage, OpVariableInitializerBad) {
+TEST_F(ValidateIdWithMessage, OpVariableInitializerIsTypeBad) {
   string spirv = kGLSL450MemoryModel + R"(
 %1 = OpTypeInt 32 1
 %2 = OpTypePointer Input %1
 %3 = OpVariable %2 Input %2)";
   CompileSuccessfully(spirv.c_str());
   EXPECT_EQ(SPV_ERROR_INVALID_ID, ValidateInstructions());
+  EXPECT_THAT(getDiagnosticString(),
+              HasSubstr("OpVariable Initializer <id> '2' is not a constant or "
+                        "module-scope variable"));
+}
+
+TEST_F(ValidateIdWithMessage, OpVariableInitializerIsFunctionVarBad) {
+  string spirv = kGLSL450MemoryModel + R"(
+%int = OpTypeInt 32 1
+%ptrint = OpTypePointer Function %int
+%ptrptrint = OpTypePointer Function %ptrint
+%void = OpTypeVoid
+%fnty = OpTypeFunction %void
+%main = OpFunction %void None %fnty
+%entry = OpLabel
+%var = OpVariable %ptrint Function
+%varinit = OpVariable %ptrptrint Function %var ; Can't initialize function variable.
+OpReturn
+OpFunctionEnd
+)";
+  CompileSuccessfully(spirv.c_str());
+  EXPECT_EQ(SPV_ERROR_INVALID_ID, ValidateInstructions());
+  EXPECT_THAT(getDiagnosticString(),
+              HasSubstr("OpVariable Initializer <id> '8' is not a constant or "
+                        "module-scope variable"));
+}
+
+TEST_F(ValidateIdWithMessage, OpVariableInitializerIsModuleVarGood) {
+  string spirv = kGLSL450MemoryModel + R"(
+%int = OpTypeInt 32 1
+%ptrint = OpTypePointer Uniform %int
+%mvar = OpVariable %ptrint Uniform
+%ptrptrint = OpTypePointer Function %ptrint
+%void = OpTypeVoid
+%fnty = OpTypeFunction %void
+%main = OpFunction %void None %fnty
+%entry = OpLabel
+%goodvar = OpVariable %ptrptrint Function %mvar ; This is ok
+OpReturn
+OpFunctionEnd
+)";
+  CompileSuccessfully(spirv.c_str());
+  EXPECT_EQ(SPV_SUCCESS, ValidateInstructions());
 }
 
 TEST_F(ValidateIdWithMessage, OpLoadGood) {
-- 
2.7.4